public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device
@ 2026-02-17  9:23 Ian Ray
  2026-02-17  9:23 ` [PATCH 2/2] hwmon: (ina2xx) Add support for INA234 Ian Ray
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ian Ray @ 2026-02-17  9:23 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Ian Ray, Krzysztof Kozlowski, linux-hwmon, devicetree,
	linux-kernel

Add a compatible string for the INA234 device, which is like INA226 but
has different scaling.

Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
---
 Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
index d3cde8936686..009d78b30859 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
@@ -29,6 +29,7 @@ properties:
       - ti,ina230
       - ti,ina231
       - ti,ina233
+      - ti,ina234
       - ti,ina237
       - ti,ina238
       - ti,ina260
@@ -113,6 +114,7 @@ allOf:
               - ti,ina228
               - ti,ina230
               - ti,ina231
+              - ti,ina234
               - ti,ina237
               - ti,ina238
               - ti,ina260
@@ -134,6 +136,7 @@ allOf:
               - ti,ina226
               - ti,ina230
               - ti,ina231
+              - ti,ina234
               - ti,ina260
               - ti,ina700
               - ti,ina780
-- 
2.49.0


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

* [PATCH 2/2] hwmon: (ina2xx) Add support for INA234
  2026-02-17  9:23 [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device Ian Ray
@ 2026-02-17  9:23 ` Ian Ray
  2026-02-17 17:37   ` kernel test robot
  2026-02-18 22:28   ` Bence Csókás
  2026-02-17 10:05 ` [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device Krzysztof Kozlowski
  2026-02-17 10:15 ` Krzysztof Kozlowski
  2 siblings, 2 replies; 8+ messages in thread
From: Ian Ray @ 2026-02-17  9:23 UTC (permalink / raw)
  To: Guenter Roeck, Jonathan Corbet, Shuah Khan
  Cc: Ian Ray, linux-hwmon, linux-doc, linux-kernel

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@gehealthcare.com>
---
 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@gmail.com>
 
 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
 ------------------------------------------------------------------------
 
 ======================= ====================================================
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 */
 #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 */
 #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 };
 
 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,
+		.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)
 				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
-- 
2.49.0


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

* Re: [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device
  2026-02-17  9:23 [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device Ian Ray
  2026-02-17  9:23 ` [PATCH 2/2] hwmon: (ina2xx) Add support for INA234 Ian Ray
@ 2026-02-17 10:05 ` Krzysztof Kozlowski
  2026-02-17 10:10   ` Ian Ray
  2026-02-17 10:15 ` Krzysztof Kozlowski
  2 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-17 10:05 UTC (permalink / raw)
  To: Ian Ray, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-hwmon, devicetree, linux-kernel

On 17/02/2026 10:23, Ian Ray wrote:
> Add a compatible string for the INA234 device, which is like INA226 but
> has different scaling.
> 
> Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
> ---
>  Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 3 +++

I got 1/2 but no cover letter, no 2/2 and no explanation in the
changelog of that. Usually I use driver code to avoid you questions
about compatibility, but since you decided not to send it to me, then
you have question:

Does "different scaling" mean devices are not compatible in terms of DT
compatibility (subset/superset)?

Best regards,
Krzysztof

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

* Re: [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device
  2026-02-17 10:05 ` [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device Krzysztof Kozlowski
@ 2026-02-17 10:10   ` Ian Ray
  0 siblings, 0 replies; 8+ messages in thread
From: Ian Ray @ 2026-02-17 10:10 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-hwmon, devicetree, linux-kernel

On Tue, Feb 17, 2026 at 11:05:47AM +0100, Krzysztof Kozlowski wrote:
> CAUTION: This email originated from outside of GE HealthCare. Only open links or attachments if you trust the sender. Report suspicious emails using Outlook’s “Report” button.
> 
> On 17/02/2026 10:23, Ian Ray wrote:
> > Add a compatible string for the INA234 device, which is like INA226 but
> > has different scaling.
> >
> > Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
> > ---
> >  Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 3 +++
> 
> I got 1/2 but no cover letter, no 2/2 and no explanation in the
> changelog of that. Usually I use driver code to avoid you questions
> about compatibility, but since you decided not to send it to me, then
> you have question:

Sincere apologies, I used scripts/get_maintainer.pl with git-send-email
and failed to notice that you were not cc'd.

Patch [2/2] is here:
https://lore.kernel.org/all/20260217092325.15867-2-ian.ray@gehealthcare.com/

> 
> Does "different scaling" mean devices are not compatible in terms of DT
> compatibility (subset/superset)?

That's correct, the device tree compatible must be different since the
driver uses the compatible to configure the scaling.

> 
> Best regards,
> Krzysztof

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

* Re: [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device
  2026-02-17  9:23 [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device Ian Ray
  2026-02-17  9:23 ` [PATCH 2/2] hwmon: (ina2xx) Add support for INA234 Ian Ray
  2026-02-17 10:05 ` [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device Krzysztof Kozlowski
@ 2026-02-17 10:15 ` Krzysztof Kozlowski
  2 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-17 10:15 UTC (permalink / raw)
  To: Ian Ray, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-hwmon, devicetree, linux-kernel

On 17/02/2026 10:23, Ian Ray wrote:
> Add a compatible string for the INA234 device, which is like INA226 but
> has different scaling.
> 
> Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
> ---
>  Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

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

* Re: [PATCH 2/2] hwmon: (ina2xx) Add support for INA234
  2026-02-17  9:23 ` [PATCH 2/2] hwmon: (ina2xx) Add support for INA234 Ian Ray
@ 2026-02-17 17:37   ` kernel test robot
  2026-02-18 22:28   ` Bence Csókás
  1 sibling, 0 replies; 8+ messages in thread
From: kernel test robot @ 2026-02-17 17:37 UTC (permalink / raw)
  To: Ian Ray, Guenter Roeck, Jonathan Corbet, Shuah Khan
  Cc: oe-kbuild-all, Ian Ray, linux-hwmon, linux-doc, linux-kernel

Hi Ian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on robh/for-next krzk-dt/for-next next-20260216]
[cannot apply to linus/master v6.16-rc1]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ian-Ray/hwmon-ina2xx-Add-support-for-INA234/20260217-172450
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20260217092325.15867-2-ian.ray%40gehealthcare.com
patch subject: [PATCH 2/2] hwmon: (ina2xx) Add support for INA234
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
docutils: docutils (Docutils 0.21.2, Python 3.13.5, on linux)
reproduce: (https://download.01.org/0day-ci/archive/20260217/202602171807.LY5U6PB6-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602171807.LY5U6PB6-lkp@intel.com/

All warnings (new ones prefixed by >>):

   Runtime Survivability
   ===================== [docutils]
>> Documentation/hwmon/ina2xx.rst:138: WARNING: Title underline too short.


vim +138 Documentation/hwmon/ina2xx.rst

9a629d7ada78c3 Documentation/hwmon/ina2xx     Nicolin Chen 2018-11-19  136  
d97d11d21ca0a7 Documentation/hwmon/ina2xx.rst Ian Ray      2026-02-17  137  Additional sysfs entries for ina226, ina230, ina231, ina234, ina260, and sy24655
52172ad87a22ed Documentation/hwmon/ina2xx.rst Wenliang Yan 2024-11-06 @138  ------------------------------------------------------------------------
9a629d7ada78c3 Documentation/hwmon/ina2xx     Nicolin Chen 2018-11-19  139  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH 2/2] hwmon: (ina2xx) Add support for INA234
  2026-02-17  9:23 ` [PATCH 2/2] hwmon: (ina2xx) Add support for INA234 Ian Ray
  2026-02-17 17:37   ` kernel test robot
@ 2026-02-18 22:28   ` Bence Csókás
       [not found]     ` <ac1ec18e-1e50-4f66-92ef-728ed3855538@Spark>
  1 sibling, 1 reply; 8+ messages in thread
From: Bence Csókás @ 2026-02-18 22:28 UTC (permalink / raw)
  To: Ian Ray, Tomaz Zaman
  Cc: linux-hwmon, linux-doc, linux-kernel, Guenter Roeck,
	Jonathan Corbet, Shuah Khan

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@gehealthcare.com>
> ---
>   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@gmail.com>
>   
>   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

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

* Re: [PATCH 2/2] hwmon: (ina2xx) Add support for INA234
       [not found]     ` <ac1ec18e-1e50-4f66-92ef-728ed3855538@Spark>
@ 2026-02-19 11:31       ` Ian Ray
  0 siblings, 0 replies; 8+ messages in thread
From: Ian Ray @ 2026-02-19 11:31 UTC (permalink / raw)
  To: Tomaž Zaman, Bence Csókás
  Cc: Bence Csókás, linux-hwmon@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Guenter Roeck, Jonathan Corbet, Shuah Khan

Note that an earlier version of the datasheet (rev C) said:
"Power [W] = 32 x CURRENT_LSB x POWER" (equation 4).

On Thu, Feb 19, 2026 at 11:15:14AM +0000, Tomaž Zaman wrote:
> 32 is the correct value, mine (25) is wrong. 
>

As Bence noted, in datasheet rev D this has now been changed (and IIUC,
the changed values are for example purposes).


> I’m curious about 25600, though, I believe it should be 1600 for bus_voltage_lsb (12 bit ADC on 16 bit regiters means 4 bit shift = 25600/16).
> 

I took this from table 7-1 of the INA234 datasheet (25.6 mV/LSB).

The voltage and current error between INA234 and the result from
multimeter is less than 3%.  So, the numbers seem to be empirically
correct (on my hardware).

> 
> 
> Tomaž Zaman, CEO 
> Mono Technologies Inc.
> 
> 
> 
> > On 18 Feb 2026 at 23:29 +0100, Bence Csókás <bence98@sch.bme.hu>, wrote:
> > 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@gehealthcare.com>
> > ---
> > 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@gmail.com>
> > 
> > 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.
> > 

Yes, makes sense.

> > 
> > ======================= ====================================================
> > 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.

INA226_DIE_ID is applicable for INA226, whereas the first 8 registers
are applicable to a wider set of chips (incuding INA234).  Perhaps I
should change the earlier comment to "INA2xx register definitions"?

> > 
> > #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`?

Sure, we can re-use that.

> > 
> > #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?

Agreed!

> > 
> > 
> > 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?

(See earlier discussion.)

> > 
> > [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.

Agreed.

> > 
> > 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

Many thanks for the reviews.

I will prepare a V2.

Regards,
Ian

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

end of thread, other threads:[~2026-02-19 11:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-17  9:23 [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device Ian Ray
2026-02-17  9:23 ` [PATCH 2/2] hwmon: (ina2xx) Add support for INA234 Ian Ray
2026-02-17 17:37   ` kernel test robot
2026-02-18 22:28   ` Bence Csókás
     [not found]     ` <ac1ec18e-1e50-4f66-92ef-728ed3855538@Spark>
2026-02-19 11:31       ` Ian Ray
2026-02-17 10:05 ` [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA234 device Krzysztof Kozlowski
2026-02-17 10:10   ` Ian Ray
2026-02-17 10:15 ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox