* [PATCH 0/3] Add INA232 power monitor support for Arduino VENTUNO Q
@ 2026-06-10 8:32 Loic Poulain
2026-06-10 8:32 ` [PATCH 1/3] dt-bindings: hwmon: ina2xx: add ina232 compatible Loic Poulain
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Loic Poulain @ 2026-06-10 8:32 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: Krzysztof Kozlowski, linux-hwmon, devicetree, linux-kernel,
linux-arm-msm, Loic Poulain, Martino Facchin
Add support for the TI INA232 current/power monitor to the ina2xx
hwmon driver, and enable it on the Arduino Monza board.
The INA232 is a bidirectional current/power monitor that shares the
same I2C register layout as the INA226, but has different electrical
characteristics.
On the Arduino Monza/Ventuno-Q board, the INA232 is connected on
I2C12 at address 0x40 with a 2 mΩ shunt resistor, and is used to
monitor the board supply current.
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
Loic Poulain (1):
arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor
Martino Facchin (2):
dt-bindings: hwmon: ina2xx: add ina232 compatible
hwmon: ina2xx: support ina232
Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 1 +
arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts | 6 ++++++
drivers/hwmon/ina2xx.c | 17 +++++++++++++++++
3 files changed, 24 insertions(+)
---
base-commit: 8a4062d204752e0d66a1e7e1a2f8834571a8d40f
change-id: 20260610-monza-ina232-de180e669dc1
Best regards,
--
Loic Poulain <loic.poulain@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] dt-bindings: hwmon: ina2xx: add ina232 compatible
2026-06-10 8:32 [PATCH 0/3] Add INA232 power monitor support for Arduino VENTUNO Q Loic Poulain
@ 2026-06-10 8:32 ` Loic Poulain
2026-06-10 8:41 ` sashiko-bot
2026-06-10 8:32 ` [PATCH 2/3] hwmon: ina2xx: support ina232 Loic Poulain
2026-06-10 8:32 ` [PATCH 3/3] arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor Loic Poulain
2 siblings, 1 reply; 11+ messages in thread
From: Loic Poulain @ 2026-06-10 8:32 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: Krzysztof Kozlowski, linux-hwmon, devicetree, linux-kernel,
linux-arm-msm, Loic Poulain, Martino Facchin
From: Martino Facchin <m.facchin@arduino.cc>
The INA232 is a current/power monitor from Texas Instruments sharing
the same register map as the other INA2xx.
Signed-off-by: Martino Facchin <m.facchin@arduino.cc>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
index 009d78b308596ca795bebdd160431bd718b127e0..a30888c9156b977671b3c48937d4ba972406ae91 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
@@ -28,6 +28,7 @@ properties:
- ti,ina228
- ti,ina230
- ti,ina231
+ - ti,ina232
- ti,ina233
- ti,ina234
- ti,ina237
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] hwmon: ina2xx: support ina232
2026-06-10 8:32 [PATCH 0/3] Add INA232 power monitor support for Arduino VENTUNO Q Loic Poulain
2026-06-10 8:32 ` [PATCH 1/3] dt-bindings: hwmon: ina2xx: add ina232 compatible Loic Poulain
@ 2026-06-10 8:32 ` Loic Poulain
2026-06-10 8:43 ` sashiko-bot
2026-06-10 8:32 ` [PATCH 3/3] arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor Loic Poulain
2 siblings, 1 reply; 11+ messages in thread
From: Loic Poulain @ 2026-06-10 8:32 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: Krzysztof Kozlowski, linux-hwmon, devicetree, linux-kernel,
linux-arm-msm, Loic Poulain, Martino Facchin
From: Martino Facchin <m.facchin@arduino.cc>
The INA232 is a current/power monitor. It shares the same register
layout as the INA2xx and uses the INA226 default configuration, but
differs in its electrical characteristics:
Signed-off-by: Martino Facchin <m.facchin@arduino.cc>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
drivers/hwmon/ina2xx.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index 613ffb622b7c42b8b6090d3b4ec7b2fa412e24a4..122e7aa4fdfffb5bac3d15ff0496fa862147f443 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -122,6 +122,7 @@ static const struct regmap_config ina2xx_regmap_config = {
enum ina2xx_ids {
ina219,
ina226,
+ ina232,
ina234,
ina260,
sy24655
@@ -196,6 +197,17 @@ static const struct ina2xx_config ina2xx_config[] = {
.current_shift = 4,
.has_update_interval = true,
},
+ [ina232] = {
+ .config_default = INA226_CONFIG_DEFAULT,
+ .calibration_value = 2048,
+ .shunt_div = 400,
+ .bus_voltage_shift = 0,
+ .bus_voltage_lsb = 1600,
+ .power_lsb_factor = 32,
+ .has_alerts = true,
+ .has_ishunt = false,
+ .has_power_average = false,
+ },
[ina260] = {
.config_default = INA260_CONFIG_DEFAULT,
.shunt_div = 400,
@@ -1005,6 +1017,7 @@ static const struct i2c_device_id ina2xx_id[] = {
{ "ina226", ina226 },
{ "ina230", ina226 },
{ "ina231", ina226 },
+ { "ina232", ina232 },
{ "ina234", ina234 },
{ "ina260", ina260 },
{ "sy24655", sy24655 },
@@ -1037,6 +1050,10 @@ static const struct of_device_id __maybe_unused ina2xx_of_match[] = {
.compatible = "ti,ina231",
.data = (void *)ina226
},
+ {
+ .compatible = "ti,ina232",
+ .data = (void *)ina232
+ },
{
.compatible = "ti,ina234",
.data = (void *)ina234
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor
2026-06-10 8:32 [PATCH 0/3] Add INA232 power monitor support for Arduino VENTUNO Q Loic Poulain
2026-06-10 8:32 ` [PATCH 1/3] dt-bindings: hwmon: ina2xx: add ina232 compatible Loic Poulain
2026-06-10 8:32 ` [PATCH 2/3] hwmon: ina2xx: support ina232 Loic Poulain
@ 2026-06-10 8:32 ` Loic Poulain
2026-06-10 8:37 ` sashiko-bot
2026-06-10 8:54 ` Konrad Dybcio
2 siblings, 2 replies; 11+ messages in thread
From: Loic Poulain @ 2026-06-10 8:32 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: Krzysztof Kozlowski, linux-hwmon, devicetree, linux-kernel,
linux-arm-msm, Loic Poulain
The Arduino Monza board has a TI INA232 current/power monitor connected
on I2C12 at address 0x40. It is used to measure the board supply current
through a 2Mohm shunt resistor.
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
index 379b796f261f4c8f6b7712c5d340b20be1b9217c..a526c8a07b30c22820f54b54644916b268496209 100644
--- a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
+++ b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
@@ -291,6 +291,12 @@ adv7535_out: endpoint {
};
};
};
+
+ power-sensor@40 {
+ compatible = "ti,ina232";
+ reg = <0x40>;
+ shunt-resistor = <2000>;
+ };
};
&mdss {
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor
2026-06-10 8:32 ` [PATCH 3/3] arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor Loic Poulain
@ 2026-06-10 8:37 ` sashiko-bot
2026-06-10 8:52 ` Loic Poulain
2026-06-10 8:54 ` Konrad Dybcio
1 sibling, 1 reply; 11+ messages in thread
From: sashiko-bot @ 2026-06-10 8:37 UTC (permalink / raw)
To: Loic Poulain; +Cc: linux-hwmon, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The commit message incorrectly states the shunt resistor is '2Mohm' (2 Mega-ohms) instead of '2mOhm' (2 milli-ohms). The device tree property `shunt-resistor = <2000>;` correctly sets the value to 2000 micro-ohms (2 milli-ohms), meaning this is a commit message typo.
--
commit e207629f293c0c8e295115a6f01dea08156bf3e5
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>
arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor
This commit adds a TI INA232 power sensor to the Arduino Monza board's
device tree. It configures the sensor on I2C12 to measure the board supply
current using a shunt resistor.
> The Arduino Monza board has a TI INA232 current/power monitor connected
> on I2C12 at address 0x40. It is used to measure the board supply current
> through a 2Mohm shunt resistor.
[Severity: Low]
This isn't a bug, but there is a typo in the commit message. The text states
2Mohm (Mega-ohm), but the device tree property shunt-resistor = <2000>
configures the hardware for 2000 micro-ohms, which is 2mOhm (milli-ohm).
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260610-monza-ina232-v1-0-925b0d12771b@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: hwmon: ina2xx: add ina232 compatible
2026-06-10 8:32 ` [PATCH 1/3] dt-bindings: hwmon: ina2xx: add ina232 compatible Loic Poulain
@ 2026-06-10 8:41 ` sashiko-bot
2026-06-10 9:01 ` Loic Poulain
0 siblings, 1 reply; 11+ messages in thread
From: sashiko-bot @ 2026-06-10 8:41 UTC (permalink / raw)
To: Loic Poulain; +Cc: robh, conor+dt, linux-hwmon, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `ti,ina232` compatible string was added to the top-level enum but omitted from the existing `if-then` conditional blocks that restrict unsupported properties.
--
commit 3cd10611f4c71e7280e19fcd316cd7a918042ee6
Author: Martino Facchin <m.facchin@arduino.cc>
dt-bindings: hwmon: ina2xx: add ina232 compatible
The INA232 is a current/power monitor from Texas Instruments sharing
the same register map as the other INA2xx.
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> index 009d78b308596..a30888c9156b9 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> @@ -28,6 +28,7 @@ properties:
> - ti,ina228
> - ti,ina230
> - ti,ina231
> + - ti,ina232
[Severity: Low]
Does this new compatible string need to be added to the allOf conditional
blocks further down in the schema file?
By omitting ti,ina232 from the exclusion list for
ti,maximum-expected-current-microamp, the schema might incorrectly allow
this property for ti,ina232 nodes:
allOf:
- if:
properties:
compatible:
contains:
enum:
- silergy,sq52206
...
- ti,ina231
- ti,ina234
- ti,ina237
then:
properties:
ti,maximum-expected-current-microamp: false
Should ti,ina232 be added alongside ti,ina231 here?
Similarly, ti,ina232 appears to be omitted from the conditional block
restricting ti,shunt-gain:
- if:
properties:
compatible:
contains:
enum:
- silergy,sy24655
...
- ti,ina231
- ti,ina234
- ti,ina260
then:
properties:
ti,shunt-gain: false
Could this weaken schema validation when checking device tree nodes for
this specific hardware variant?
> - ti,ina233
> - ti,ina234
> - ti,ina237
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260610-monza-ina232-v1-0-925b0d12771b@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] hwmon: ina2xx: support ina232
2026-06-10 8:32 ` [PATCH 2/3] hwmon: ina2xx: support ina232 Loic Poulain
@ 2026-06-10 8:43 ` sashiko-bot
2026-06-10 9:26 ` Loic Poulain
0 siblings, 1 reply; 11+ messages in thread
From: sashiko-bot @ 2026-06-10 8:43 UTC (permalink / raw)
To: Loic Poulain; +Cc: linux-hwmon, robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] The INA232 configuration implicitly omits `.has_update_interval = true`, silently disabling sysfs update interval configuration.
Pre-existing issues:
- [High] Multiple alert limit calculations in `ina226_alert_to_reg` suffer from integer overflows and signedness mismatches on 32-bit systems, causing hardware limits to be incorrectly programmed.
- [High] Integer overflow in power calculations inside `ina2xx_get_value()` incorrectly returns negative power measurements at higher loads.
--
commit 553d30269962c93d687fffb0c6e1dcc15cbdf020
Author: Martino Facchin <m.facchin@arduino.cc>
Subject: hwmon: ina2xx: support ina232
This commit adds support for the INA232 current and power monitor. It defines
the INA232 configuration values and adds the device to the corresponding i2c
and device tree identification tables.
> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> index 613ffb622b7c4..122e7aa4fdfff 100644
> --- a/drivers/hwmon/ina2xx.c
> +++ b/drivers/hwmon/ina2xx.c
[ ... ]
> @@ -196,6 +197,17 @@ static const struct ina2xx_config ina2xx_config[] = {
> .current_shift = 4,
> .has_update_interval = true,
> },
> + [ina232] = {
> + .config_default = INA226_CONFIG_DEFAULT,
> + .calibration_value = 2048,
> + .shunt_div = 400,
> + .bus_voltage_shift = 0,
> + .bus_voltage_lsb = 1600,
> + .power_lsb_factor = 32,
[Severity: High]
This is a pre-existing issue, but can the power calculation overflow when
using larger power_lsb_factor values like the one added here for INA232?
In ina2xx_get_value(), the INA2XX_POWER calculation is:
val = regval * data->power_lsb_uW;
The result is returned as a signed 32-bit int. For the INA232 with a common
2mOhm shunt, power_lsb_uW becomes 40,000. When the 16-bit regval exceeds
53,687, the product exceeds INT_MAX. This overflows the 32-bit signed math,
wrapping to a negative integer and reporting erroneous negative power
readings to userspace.
[Severity: High]
This is also a pre-existing issue, but do the alert limit calculations in
ina226_alert_to_reg() cause hardware limits to be incorrectly programmed on
32-bit systems?
For INA2XX_POWER, the upper bound is clamped:
val = clamp_val(val, 0, UINT_MAX - data->power_lsb_uW);
clamp_val() implicitly casts the upper bound to a 32-bit signed long on
32-bit platforms. This results in a negative number, meaning any valid
power limit is clamped to a negative value and ultimately programmed as 0.
Similarly, for INA2XX_SHUNT_VOLTAGE, the initial clamp uses SHRT_MAX *
shunt_div instead of division. The subsequent multiplication:
val *= data->config->shunt_div;
overflows LONG_MAX on 32-bit platforms for high inputs, wrapping to a
negative value and also programming the hardware limit to 0.
Additionally, for INA2XX_BUS_VOLTAGE on parts with bus_voltage_shift > 0,
the calculation (val * 1000) << shift can exceed LONG_MAX for limits over
~134V, wrapping to negative and setting the limit to 0.
> + .has_alerts = true,
> + .has_ishunt = false,
> + .has_power_average = false,
> + },
[Severity: Medium]
Does this configuration intentionally omit the update interval?
Because has_update_interval is not explicitly set, it defaults to false.
This hides the hwmon_chip_update_interval sysfs attribute, preventing
adjustments to the averaging time despite the INA232 sharing the INA226
register layout.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260610-monza-ina232-v1-0-925b0d12771b@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor
2026-06-10 8:37 ` sashiko-bot
@ 2026-06-10 8:52 ` Loic Poulain
0 siblings, 0 replies; 11+ messages in thread
From: Loic Poulain @ 2026-06-10 8:52 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-hwmon, devicetree, robh, conor+dt
On Wed, Jun 10, 2026 at 10:37 AM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The commit message incorrectly states the shunt resistor is '2Mohm' (2 Mega-ohms) instead of '2mOhm' (2 milli-ohms). The device tree property `shunt-resistor = <2000>;` correctly sets the value to 2000 micro-ohms (2 milli-ohms), meaning this is a commit message typo.
> --
>
> commit e207629f293c0c8e295115a6f01dea08156bf3e5
> Author: Loic Poulain <loic.poulain@oss.qualcomm.com>
>
> arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor
>
> This commit adds a TI INA232 power sensor to the Arduino Monza board's
> device tree. It configures the sensor on I2C12 to measure the board supply
> current using a shunt resistor.
>
> > The Arduino Monza board has a TI INA232 current/power monitor connected
> > on I2C12 at address 0x40. It is used to measure the board supply current
> > through a 2Mohm shunt resistor.
>
> [Severity: Low]
> This isn't a bug, but there is a typo in the commit message. The text states
> 2Mohm (Mega-ohm), but the device tree property shunt-resistor = <2000>
> configures the hardware for 2000 micro-ohms, which is 2mOhm (milli-ohm).
It's indeed a 2milliohm (mOhm) resistor.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor
2026-06-10 8:32 ` [PATCH 3/3] arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor Loic Poulain
2026-06-10 8:37 ` sashiko-bot
@ 2026-06-10 8:54 ` Konrad Dybcio
1 sibling, 0 replies; 11+ messages in thread
From: Konrad Dybcio @ 2026-06-10 8:54 UTC (permalink / raw)
To: Loic Poulain, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: Krzysztof Kozlowski, linux-hwmon, devicetree, linux-kernel,
linux-arm-msm
On 6/10/26 10:32 AM, Loic Poulain wrote:
> The Arduino Monza board has a TI INA232 current/power monitor connected
> on I2C12 at address 0x40. It is used to measure the board supply current
> through a 2Mohm shunt resistor.
>
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> index 379b796f261f4c8f6b7712c5d340b20be1b9217c..a526c8a07b30c22820f54b54644916b268496209 100644
> --- a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> +++ b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> @@ -291,6 +291,12 @@ adv7535_out: endpoint {
> };
> };
> };
> +
> + power-sensor@40 {
> + compatible = "ti,ina232";
> + reg = <0x40>;
> + shunt-resistor = <2000>;
The commit message says:
"through a 2Mohm shunt resistor"
and the bindings say:
"Shunt resistor value in micro-Ohm"
Konrad
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: hwmon: ina2xx: add ina232 compatible
2026-06-10 8:41 ` sashiko-bot
@ 2026-06-10 9:01 ` Loic Poulain
0 siblings, 0 replies; 11+ messages in thread
From: Loic Poulain @ 2026-06-10 9:01 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, conor+dt, linux-hwmon, devicetree
On Wed, Jun 10, 2026 at 10:41 AM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The `ti,ina232` compatible string was added to the top-level enum but omitted from the existing `if-then` conditional blocks that restrict unsupported properties.
> --
>
> commit 3cd10611f4c71e7280e19fcd316cd7a918042ee6
> Author: Martino Facchin <m.facchin@arduino.cc>
>
> dt-bindings: hwmon: ina2xx: add ina232 compatible
>
> The INA232 is a current/power monitor from Texas Instruments sharing
> the same register map as the other INA2xx.
>
> > diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> > index 009d78b308596..a30888c9156b9 100644
> > --- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> > @@ -28,6 +28,7 @@ properties:
> > - ti,ina228
> > - ti,ina230
> > - ti,ina231
> > + - ti,ina232
>
> [Severity: Low]
> Does this new compatible string need to be added to the allOf conditional
> blocks further down in the schema file?
>
> By omitting ti,ina232 from the exclusion list for
> ti,maximum-expected-current-microamp, the schema might incorrectly allow
> this property for ti,ina232 nodes:
>
> allOf:
> - if:
> properties:
> compatible:
> contains:
> enum:
> - silergy,sq52206
> ...
> - ti,ina231
> - ti,ina234
> - ti,ina237
> then:
> properties:
> ti,maximum-expected-current-microamp: false
>
> Should ti,ina232 be added alongside ti,ina231 here?
>
> Similarly, ti,ina232 appears to be omitted from the conditional block
> restricting ti,shunt-gain:
>
> - if:
> properties:
> compatible:
> contains:
> enum:
> - silergy,sy24655
> ...
> - ti,ina231
> - ti,ina234
> - ti,ina260
> then:
> properties:
> ti,shunt-gain: false
>
> Could this weaken schema validation when checking device tree nodes for
> this specific hardware variant?
That's a valid point, will be addressed in v2.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] hwmon: ina2xx: support ina232
2026-06-10 8:43 ` sashiko-bot
@ 2026-06-10 9:26 ` Loic Poulain
0 siblings, 0 replies; 11+ messages in thread
From: Loic Poulain @ 2026-06-10 9:26 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-hwmon, robh, conor+dt, devicetree
On Wed, Jun 10, 2026 at 10:44 AM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [Medium] The INA232 configuration implicitly omits `.has_update_interval = true`, silently disabling sysfs update interval configuration.
>
> Pre-existing issues:
> - [High] Multiple alert limit calculations in `ina226_alert_to_reg` suffer from integer overflows and signedness mismatches on 32-bit systems, causing hardware limits to be incorrectly programmed.
> - [High] Integer overflow in power calculations inside `ina2xx_get_value()` incorrectly returns negative power measurements at higher loads.
> --
>
> commit 553d30269962c93d687fffb0c6e1dcc15cbdf020
> Author: Martino Facchin <m.facchin@arduino.cc>
> Subject: hwmon: ina2xx: support ina232
>
> This commit adds support for the INA232 current and power monitor. It defines
> the INA232 configuration values and adds the device to the corresponding i2c
> and device tree identification tables.
>
> > diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> > index 613ffb622b7c4..122e7aa4fdfff 100644
> > --- a/drivers/hwmon/ina2xx.c
> > +++ b/drivers/hwmon/ina2xx.c
>
> [ ... ]
>
> > @@ -196,6 +197,17 @@ static const struct ina2xx_config ina2xx_config[] = {
> > .current_shift = 4,
> > .has_update_interval = true,
> > },
> > + [ina232] = {
> > + .config_default = INA226_CONFIG_DEFAULT,
> > + .calibration_value = 2048,
> > + .shunt_div = 400,
> > + .bus_voltage_shift = 0,
> > + .bus_voltage_lsb = 1600,
> > + .power_lsb_factor = 32,
>
> [Severity: High]
> This is a pre-existing issue, but can the power calculation overflow when
> using larger power_lsb_factor values like the one added here for INA232?
>
> In ina2xx_get_value(), the INA2XX_POWER calculation is:
>
> val = regval * data->power_lsb_uW;
>
> The result is returned as a signed 32-bit int. For the INA232 with a common
> 2mOhm shunt, power_lsb_uW becomes 40,000. When the 16-bit regval exceeds
> 53,687, the product exceeds INT_MAX. This overflows the 32-bit signed math,
> wrapping to a negative integer and reporting erroneous negative power
> readings to userspace.
>
> [Severity: High]
> This is also a pre-existing issue, but do the alert limit calculations in
> ina226_alert_to_reg() cause hardware limits to be incorrectly programmed on
> 32-bit systems?
>
> For INA2XX_POWER, the upper bound is clamped:
>
> val = clamp_val(val, 0, UINT_MAX - data->power_lsb_uW);
>
> clamp_val() implicitly casts the upper bound to a 32-bit signed long on
> 32-bit platforms. This results in a negative number, meaning any valid
> power limit is clamped to a negative value and ultimately programmed as 0.
>
> Similarly, for INA2XX_SHUNT_VOLTAGE, the initial clamp uses SHRT_MAX *
> shunt_div instead of division. The subsequent multiplication:
>
> val *= data->config->shunt_div;
>
> overflows LONG_MAX on 32-bit platforms for high inputs, wrapping to a
> negative value and also programming the hardware limit to 0.
>
> Additionally, for INA2XX_BUS_VOLTAGE on parts with bus_voltage_shift > 0,
> the calculation (val * 1000) << shift can exceed LONG_MAX for limits over
> ~134V, wrapping to negative and setting the limit to 0.
>
> > + .has_alerts = true,
> > + .has_ishunt = false,
> > + .has_power_average = false,
> > + },
>
> [Severity: Medium]
> Does this configuration intentionally omit the update interval?
>
> Because has_update_interval is not explicitly set, it defaults to false.
> This hides the hwmon_chip_update_interval sysfs attribute, preventing
> adjustments to the averaging time despite the INA232 sharing the INA226
> register layout.
These are pre-existing issues related to out-of-scope voltage and
power values. They will not be addressed in this series.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-06-10 9:26 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-10 8:32 [PATCH 0/3] Add INA232 power monitor support for Arduino VENTUNO Q Loic Poulain
2026-06-10 8:32 ` [PATCH 1/3] dt-bindings: hwmon: ina2xx: add ina232 compatible Loic Poulain
2026-06-10 8:41 ` sashiko-bot
2026-06-10 9:01 ` Loic Poulain
2026-06-10 8:32 ` [PATCH 2/3] hwmon: ina2xx: support ina232 Loic Poulain
2026-06-10 8:43 ` sashiko-bot
2026-06-10 9:26 ` Loic Poulain
2026-06-10 8:32 ` [PATCH 3/3] arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor Loic Poulain
2026-06-10 8:37 ` sashiko-bot
2026-06-10 8:52 ` Loic Poulain
2026-06-10 8:54 ` Konrad Dybcio
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox