[PATCH 0/2] hwmon: ina2xx conistency improvements

[PATCH 0/2] hwmon: ina2xx conistency improvements

[Jonas Rebmann]

While working on INA234 support in the ina2xx.c, I think I found some
inconsistencies: An unused macro, outdated comments, and different
approaches to accounting for zero-reserved LSBs in INA234 registers.

These aren't functional changes and it was verified that measurements
are correct as before on INA234.

Ian Ray <ian.ray@gehealthcare.com>

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Jonas Rebmann (2):
      hwmon: (ina2xx) clean up unused macro and outdated comment
      hwmon: (ina2xx) Shift INA234 shunt and current registers

 drivers/hwmon/ina2xx.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)
---
base-commit: f08c5de5f61a117ba5326d3d5b86e884077da2d0
change-id: 20260303-ina234-shift-ca6be45f4a7d

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

[PATCH 1/2] hwmon: (ina2xx) clean up unused macro and outdated comment

[Jonas Rebmann]

The list of supported chips in the header is incomplete and contains no
other information not readily available. Remove the list and instead
hint that the chips supported by this driver have 219/226 compatible
register layout [unlike the ones supported by e.g. ina238].

Remove the unused INA226_DIE_ID macro.

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

diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index 836e15a5a780..6a2cebbb9f15 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -1,22 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Driver for Texas Instruments INA219, INA226 power monitor chips
- *
- * INA219:
- * Zero Drift Bi-Directional Current/Power Monitor with I2C Interface
- * Datasheet: https://www.ti.com/product/ina219
- *
- * INA220:
- * Bi-Directional Current/Power Monitor with I2C Interface
- * Datasheet: https://www.ti.com/product/ina220
- *
- * INA226:
- * Bi-Directional Current/Power Monitor with I2C Interface
- * Datasheet: https://www.ti.com/product/ina226
- *
- * INA230:
- * Bi-directional Current/Power Monitor with I2C Interface
- * Datasheet: https://www.ti.com/product/ina230
+ * Driver for Texas Instruments INA219, INA226 and register-layout compatible
+ * current/power monitor chips with I2C Interface
  *
  * Copyright (C) 2012 Lothar Felten <lothar.felten@gmail.com>
  * Thanks to Jan Volkering
@@ -49,7 +34,6 @@
 /* INA226 register definitions */
 #define INA226_MASK_ENABLE		0x06
 #define INA226_ALERT_LIMIT		0x07
-#define INA226_DIE_ID			0xFF
 
 /* SY24655 register definitions */
 #define SY24655_EIN				0x0A

-- 
2.51.2.535.g419c72cb8a

[PATCH 2/2] hwmon: (ina2xx) Shift INA234 shunt and current registers

[Jonas Rebmann]

The INA219 has the lowest three bits of the bus voltage register
zero-reserved, the bus_voltage_shift ina2xx_config field was introduced
to accommodate for that.

The INA234 has four bits of the bus voltage, of the shunt voltage, and
of the current registers zero-reserved but the latter two were
implemented by choosing a 16x higher shunt_div instead of a separate
field specifying a bit shift.

This is possible because shunt voltage and current are divided by
shunt_div, hence a 16x higher shunt_div results in a 16x smaller LSB for
both the shunt voltage and the current register, perfectly accounting
for the missing bit shift.

For consistency and correctness, account for the reserved bits via
shunt_voltage_shift and current_shift configuration fields as already
done for voltage registers and use the conversion constants given in the
INA234 datasheet.

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

diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index 6a2cebbb9f15..613ffb622b7c 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -135,9 +135,11 @@ struct ina2xx_config {
 	bool has_update_interval;
 	int calibration_value;
 	int shunt_div;
+	int shunt_voltage_shift;
 	int bus_voltage_shift;
 	int bus_voltage_lsb;	/* uV */
 	int power_lsb_factor;
+	int current_shift;
 };
 
 struct ina2xx_data {
@@ -156,59 +158,69 @@ static const struct ina2xx_config ina2xx_config[] = {
 		.config_default = INA219_CONFIG_DEFAULT,
 		.calibration_value = 4096,
 		.shunt_div = 100,
+		.shunt_voltage_shift = 0,
 		.bus_voltage_shift = 3,
 		.bus_voltage_lsb = 4000,
 		.power_lsb_factor = 20,
 		.has_alerts = false,
 		.has_ishunt = false,
 		.has_power_average = false,
+		.current_shift = 0,
 		.has_update_interval = false,
 	},
 	[ina226] = {
 		.config_default = INA226_CONFIG_DEFAULT,
 		.calibration_value = 2048,
 		.shunt_div = 400,
+		.shunt_voltage_shift = 0,
 		.bus_voltage_shift = 0,
 		.bus_voltage_lsb = 1250,
 		.power_lsb_factor = 25,
 		.has_alerts = true,
 		.has_ishunt = false,
 		.has_power_average = false,
+		.current_shift = 0,
 		.has_update_interval = true,
 	},
 	[ina234] = {
 		.config_default = INA226_CONFIG_DEFAULT,
 		.calibration_value = 2048,
-		.shunt_div = 400, /* 2.5 µV/LSB raw ADC reading from INA2XX_SHUNT_VOLTAGE */
+		.shunt_div = 25, /* 2.5 µV/LSB raw ADC reading from INA2XX_SHUNT_VOLTAGE */
+		.shunt_voltage_shift = 4,
 		.bus_voltage_shift = 4,
 		.bus_voltage_lsb = 25600,
 		.power_lsb_factor = 32,
 		.has_alerts = true,
 		.has_ishunt = false,
 		.has_power_average = false,
+		.current_shift = 4,
 		.has_update_interval = true,
 	},
 	[ina260] = {
 		.config_default = INA260_CONFIG_DEFAULT,
 		.shunt_div = 400,
+		.shunt_voltage_shift = 0,
 		.bus_voltage_shift = 0,
 		.bus_voltage_lsb = 1250,
 		.power_lsb_factor = 8,
 		.has_alerts = true,
 		.has_ishunt = true,
 		.has_power_average = false,
+		.current_shift = 0,
 		.has_update_interval = true,
 	},
 	[sy24655] = {
 		.config_default = SY24655_CONFIG_DEFAULT,
 		.calibration_value = 4096,
 		.shunt_div = 400,
+		.shunt_voltage_shift = 0,
 		.bus_voltage_shift = 0,
 		.bus_voltage_lsb = 1250,
 		.power_lsb_factor = 25,
 		.has_alerts = true,
 		.has_ishunt = false,
 		.has_power_average = true,
+		.current_shift = 0,
 		.has_update_interval = false,
 	},
 };
@@ -262,7 +274,8 @@ static int ina2xx_get_value(struct ina2xx_data *data, u8 reg,
 	switch (reg) {
 	case INA2XX_SHUNT_VOLTAGE:
 		/* signed register */
-		val = DIV_ROUND_CLOSEST((s16)regval, data->config->shunt_div);
+		val = (s16)regval >> data->config->shunt_voltage_shift;
+		val = DIV_ROUND_CLOSEST(val, data->config->shunt_div);
 		break;
 	case INA2XX_BUS_VOLTAGE:
 		val = (regval >> data->config->bus_voltage_shift) *
@@ -274,7 +287,8 @@ static int ina2xx_get_value(struct ina2xx_data *data, u8 reg,
 		break;
 	case INA2XX_CURRENT:
 		/* signed register, result in mA */
-		val = (s16)regval * data->current_lsb_uA;
+		val = ((s16)regval >> data->config->current_shift) *
+		  data->current_lsb_uA;
 		val = DIV_ROUND_CLOSEST(val, 1000);
 		break;
 	case INA2XX_CALIBRATION:
@@ -368,6 +382,7 @@ static u16 ina226_alert_to_reg(struct ina2xx_data *data, int reg, long val)
 	case INA2XX_SHUNT_VOLTAGE:
 		val = clamp_val(val, 0, SHRT_MAX * data->config->shunt_div);
 		val *= data->config->shunt_div;
+		val <<= data->config->shunt_voltage_shift;
 		return clamp_val(val, 0, SHRT_MAX);
 	case INA2XX_BUS_VOLTAGE:
 		val = clamp_val(val, 0, 200000);
@@ -382,6 +397,7 @@ static u16 ina226_alert_to_reg(struct ina2xx_data *data, int reg, long val)
 		val = clamp_val(val, INT_MIN / 1000, INT_MAX / 1000);
 		/* signed register, result in mA */
 		val = DIV_ROUND_CLOSEST(val * 1000, data->current_lsb_uA);
+		val <<= data->config->current_shift;
 		return clamp_val(val, SHRT_MIN, SHRT_MAX);
 	default:
 		/* programmer goofed */

-- 
2.51.2.535.g419c72cb8a

Re: [PATCH 1/2] hwmon: (ina2xx) clean up unused macro and outdated comment

[Guenter Roeck]

On Tue, Mar 03, 2026 at 12:07:01PM +0100, Jonas Rebmann wrote:
> The list of supported chips in the header is incomplete and contains no
> other information not readily available. Remove the list and instead
> hint that the chips supported by this driver have 219/226 compatible
> register layout [unlike the ones supported by e.g. ina238].
> 
> Remove the unused INA226_DIE_ID macro.

It is a define, not a macro. I'll change that in the commit message.

> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/ina2xx.c | 20 ++------------------
>  1 file changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> index 836e15a5a780..6a2cebbb9f15 100644
> --- a/drivers/hwmon/ina2xx.c
> +++ b/drivers/hwmon/ina2xx.c
> @@ -1,22 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Driver for Texas Instruments INA219, INA226 power monitor chips
> - *
> - * INA219:
> - * Zero Drift Bi-Directional Current/Power Monitor with I2C Interface
> - * Datasheet: https://www.ti.com/product/ina219
> - *
> - * INA220:
> - * Bi-Directional Current/Power Monitor with I2C Interface
> - * Datasheet: https://www.ti.com/product/ina220
> - *
> - * INA226:
> - * Bi-Directional Current/Power Monitor with I2C Interface
> - * Datasheet: https://www.ti.com/product/ina226
> - *
> - * INA230:
> - * Bi-directional Current/Power Monitor with I2C Interface
> - * Datasheet: https://www.ti.com/product/ina230
> + * Driver for Texas Instruments INA219, INA226 and register-layout compatible
> + * current/power monitor chips with I2C Interface
>   *
>   * Copyright (C) 2012 Lothar Felten <lothar.felten@gmail.com>
>   * Thanks to Jan Volkering
> @@ -49,7 +34,6 @@
>  /* INA226 register definitions */
>  #define INA226_MASK_ENABLE		0x06
>  #define INA226_ALERT_LIMIT		0x07
> -#define INA226_DIE_ID			0xFF
>  
>  /* SY24655 register definitions */
>  #define SY24655_EIN				0x0A

Re: [PATCH 2/2] hwmon: (ina2xx) Shift INA234 shunt and current registers

[Guenter Roeck]

On Tue, Mar 03, 2026 at 12:07:02PM +0100, Jonas Rebmann wrote:
> The INA219 has the lowest three bits of the bus voltage register
> zero-reserved, the bus_voltage_shift ina2xx_config field was introduced
> to accommodate for that.
> 
> The INA234 has four bits of the bus voltage, of the shunt voltage, and
> of the current registers zero-reserved but the latter two were
> implemented by choosing a 16x higher shunt_div instead of a separate
> field specifying a bit shift.
> 
> This is possible because shunt voltage and current are divided by
> shunt_div, hence a 16x higher shunt_div results in a 16x smaller LSB for
> both the shunt voltage and the current register, perfectly accounting
> for the missing bit shift.
> 
> For consistency and correctness, account for the reserved bits via
> shunt_voltage_shift and current_shift configuration fields as already
> done for voltage registers and use the conversion constants given in the
> INA234 datasheet.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>

Applied.

Thanks,
Guenter