Re: [PATCH 2/2] hwmon: (ina2xx) Add support for INA234
From: Bence Csókás
Date: Wed Feb 18 2026 - 17:29:37 EST
Hi Ian,
thanks for this patch!
CC'ing Tomaz who has a downstream patch for this part as well.
On 2/17/26 10:23, Ian Ray wrote:
INA234 is register compatible to INA226 (excepting manufacturer and die
or device id registers) but has different scaling.
While the manufacturer and die/device id registers are different, these
are currently unused. Comment INA226_DIE_ID to aid future maintenance.
Signed-off-by: Ian Ray <ian.ray@xxxxxxxxxxxxxxxx>
---
Documentation/hwmon/ina2xx.rst | 14 ++++++++++++--
drivers/hwmon/Kconfig | 2 +-
drivers/hwmon/ina2xx.c | 21 +++++++++++++++++++--
3 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/Documentation/hwmon/ina2xx.rst b/Documentation/hwmon/ina2xx.rst
index a3860aae444c..4c05bd5e24fb 100644
--- a/Documentation/hwmon/ina2xx.rst
+++ b/Documentation/hwmon/ina2xx.rst
@@ -74,6 +74,16 @@ Supported chips:
https://us1.silergy.com/
+ * Texas Instruments INA234
+
+ Prefix: 'ina234'
+
+ Addresses: I2C 0x40 - 0x43
+
+ Datasheet: Publicly available at the Texas Instruments website
+
+ https://www.ti.com/
+
Author: Lothar Felten <lothar.felten@xxxxxxxxx>
Description
@@ -89,7 +99,7 @@ interface. The INA220 monitors both shunt drop and supply voltage.
The INA226 is a current shunt and power monitor with an I2C interface.
The INA226 monitors both a shunt voltage drop and bus supply voltage.
-INA230 and INA231 are high or low side current shunt and power monitors
+INA230, INA231, and INA234 are high or low side current shunt and power monitors
with an I2C interface. The chips monitor both a shunt voltage drop and
bus supply voltage.
@@ -124,7 +134,7 @@ power1_input Power(uW) measurement channel
shunt_resistor Shunt resistance(uOhm) channel (not for ina260)
======================= ===============================================
-Additional sysfs entries for ina226, ina230, ina231, ina260, and sy24655
+Additional sysfs entries for ina226, ina230, ina231, ina234, ina260, and sy24655
------------------------------------------------------------------------
As buildbot already complained: you need to match the dashes' length to the text. Besides, I'm not sure that listing everything here is the best approach. I would change it to something like
Additional sysfs entries for some supported parts
And then maybe list those parts in a bullet list or something. That way, we only need to add lines going forward.
======================= ====================================================
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 41c381764c2b..6aa8a89f4747 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2284,7 +2284,7 @@ config SENSORS_INA2XX
select REGMAP_I2C
help
If you say yes here you get support for INA219, INA220, INA226,
- INA230, INA231, INA260, and SY24655 power monitor chips.
+ INA230, INA231, INA234, INA260, and SY24655 power monitor chips.
The INA2xx driver is configured for the default configuration of
the part as described in the datasheet.
diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index 69ac0468dee4..923f8c953e8f 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -49,6 +49,8 @@
/* INA226 register definitions */
#define INA226_MASK_ENABLE 0x06
#define INA226_ALERT_LIMIT 0x07
+
+/* INA226-specific register definitions */
Isn't this comment redundant? Almost the same comment is already there 3 lines above.
#define INA226_DIE_ID 0xFF
/* SY24655 register definitions */
@@ -59,6 +61,7 @@
/* settings - depend on use case */
#define INA219_CONFIG_DEFAULT 0x399F /* PGA=8 */
#define INA226_CONFIG_DEFAULT 0x4527 /* averages=16 */
+#define INA234_CONFIG_DEFAULT 0x4527 /* averages=16 */
Do we need a new macro? Wouldn't it make sense to just use `INA226_CONFIG_DEFAULT`?
#define INA260_CONFIG_DEFAULT 0x6527 /* averages=16 */
#define SY24655_CONFIG_DEFAULT 0x4527 /* averages=16 */
@@ -135,7 +138,7 @@ static const struct regmap_config ina2xx_regmap_config = {
.writeable_reg = ina2xx_writeable_reg,
};
-enum ina2xx_ids { ina219, ina226, ina260, sy24655 };
+enum ina2xx_ids { ina219, ina226, ina234, ina260, sy24655 };
Maybe it is time to break this into a multi-line enum?
struct ina2xx_config {
u16 config_default;
@@ -204,6 +207,15 @@ static const struct ina2xx_config ina2xx_config[] = {
.has_ishunt = false,
.has_power_average = true,
},
+ [ina234] = {
+ .config_default = INA234_CONFIG_DEFAULT,
+ .calibration_value = 2048,
+ .shunt_div = 400, /* 2.5 µV/LSB raw ADC reading from INA2XX_SHUNT_VOLTAGE */
+ .bus_voltage_shift = 4,
+ .bus_voltage_lsb = 25600,
+ .power_lsb_factor = 32,
How did you derive this? According to "7.1.2 Current and Power Calculations" [1] in the datasheet, `POWER_LSB = 2 * CURRENT_LSB`, so I'd think this should be `= 2`, although I'll say I'm not familiar with the IC itself. Tomaz, I do believe you had `25` here, was that just a placeholder?
[1] https://www.ti.com/lit/ds/symlink/ina234.pdf#%5B%7B%22num%22%3A421%2C%22gen%22%3A0%7D%2C%7B%22name%22%3A%22XYZ%22%7D%2Cnull%2C316.653%2Cnull%5D
+ .has_alerts = true,
+ },
};
/*
@@ -768,7 +780,7 @@ static umode_t ina2xx_is_visible(const void *_data, enum hwmon_sensor_types type
case hwmon_chip:
switch (attr) {
case hwmon_chip_update_interval:
- if (chip == ina226 || chip == ina260)
+ if (chip == ina226 || chip == ina234 || chip == ina260)
I'd say this deserves a new `has_*` member.
return 0644;
break;
default:
@@ -982,6 +994,7 @@ static const struct i2c_device_id ina2xx_id[] = {
{ "ina226", ina226 },
{ "ina230", ina226 },
{ "ina231", ina226 },
+ { "ina234", ina234 },
{ "ina260", ina260 },
{ "sy24655", sy24655 },
{ }
@@ -1013,6 +1026,10 @@ static const struct of_device_id __maybe_unused ina2xx_of_match[] = {
.compatible = "ti,ina231",
.data = (void *)ina226
},
+ {
+ .compatible = "ti,ina234",
+ .data = (void *)ina234
+ },
{
.compatible = "ti,ina260",
.data = (void *)ina260
Bence