Re: [PATCH v5 2/2] hwmon: Add support for TI INA4230 power monitor

[Guenter Roeck <linux@roeck-us.net>]

On 3/30/26 08:14, Alexey Charkov wrote:
> Add a driver for the TI INA4230, a 4-channel power monitor with I2C
> interface.
> 
> The driver supports voltage, current, power and energy measurements, but
> skips the alert functionality in this initial implementation.
> 
> Signed-off-by: Alexey Charkov <alchark@flipper.net>

Sashiko report is at

https://sashiko.dev/#/patchset/20260330-ina4230-v5-0-eeb322d95b3a%40flipper.net

Valid concerns, as far as I can see, are:

- There are various overflow issues. Please address, either by making sure that
   the operations can not overflow, or that all parameters such as the shunt
   resistor value or the interval are bound such that an overflow can not occur.
   This includes implicit conversions. For example, the interval passed to
   ina4230_interval_ms_to_conv_time() is int, but the parameter is actually long.
   There is not even a signed check, meaning the resulting interval can be pretty
   much anything.

- power is reported by the chip as unsigned value. Yet, it is converted via type
   cast to int16_t.

- Please add a comment to the energy reading to confirm that regmap_noinc_read()
   performs as expected.

- The definition of ina4230_curr_reg[] is wasteful. There is only an entry for
   hwmon_curr_input. Why specify an unnecessary two-dimensional array ?

- The dummy channel 0 for voltage is not acceptable. Make it 0-based as expected
   by the ABI. Yes, I know, but that is how the ABI was defined.


I can not comment on Saskiko's pm related feedback. PM is and has always been
a mystery to me, so I just assume that it is WAI and that there are no problems.
I do assume that you have tested the code thoroughly across suspend/resume cycles
to make sure that it works as intended.

Thanks,
Guenter