Re: [PATCH v3] iio: adc: ina2xx: add INA236 support

Jonathan Cameron
2026-03-21T19:45:13Z

On Mon, 16 Mar 2026 02:23:04 +0800
Chuang Zhu <git@chuang.cz> wrote:

> The calibration divisor is not directly specified in the datasheet, but can be calculated:
>
> I = Current_LSB * Current
> Current = ShuntVoltage * CAL / calibration_divisor
> CAL = 0.00512 / (Current_LSB * Rshunt)
> ShuntVoltage = Vshunt / ShuntVoltage_LSB
>
> => I = (0.00512 / (calibration_divisor*ShuntVoltage_LSB)) * (Vshunt / Rshunt)
>
> Ohm's law, I = Vshunt / Rshunt
> => 0.00512 / (calibration_divisor*ShuntVoltage_LSB) = 1
>
> ShuntVoltage_LSB = 2.5 uV = 0.0000025 V
> => calibration_divisor = 2048
>
> Signed-off-by: Chuang Zhu <git@chuang.cz>
> ---

No change log? There should be one here so reviewers know what you've updated
without having to go look at earlier versions.

I've picked this up with the following diff merged in:

diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index 6a4ee833d12c..dd37109a008a 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -748,20 +748,22 @@ static int ina2xx_conversion_ready(struct iio_dev *indio_dev)
case ina236:
ret = regmap_read(chip->regmap,
INA226_MASK_ENABLE, &alert);
+ if (ret < 0)
+ return ret;
+
alert &= INA226_CVRF;
break;
case ina219:
ret = regmap_read(chip->regmap,
INA2XX_BUS_VOLTAGE, &alert);
+ if (ret < 0)
+ return ret;
alert &= INA219_CNVR;
break;
default:
- ret = -EINVAL;
+ return -EINVAL;
}

- if (ret < 0)
- return ret;
-
return !!alert;
}

See below for why. Pushed out as the testing branch of iio.git.
Please give it a quick look to check I didn't mess this up!

Thanks,

J

> drivers/iio/adc/ina2xx-adc.c | 57 ++++++++++++++++++++++++++++++------
> 1 file changed, 48 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index 857e1b69d6cd..6a4ee833d12c 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -121,7 +121,7 @@ static const struct regmap_config ina2xx_regmap_config = {
> .volatile_reg = ina2xx_is_volatile_reg,
> };
>
> -enum ina2xx_ids { ina219, ina226 };
> +enum ina2xx_ids { ina219, ina226, ina236 };
>
> struct ina2xx_config {
> const char *name;
> @@ -175,6 +175,16 @@ static const struct ina2xx_config ina2xx_config[] = {
> .power_lsb_factor = 25,
> .chip_id = ina226,
> },
> + [ina236] = {
> + .name = "ina236",
> + .config_default = INA226_CONFIG_DEFAULT,
> + .calibration_value = 2048,
> + .shunt_voltage_lsb = 2500,
> + .bus_voltage_shift = 0,
> + .bus_voltage_lsb = 1600,
> + .power_lsb_factor = 32,
> + .chip_id = ina236,
> + },
> };
>
> static int ina2xx_read_raw(struct iio_dev *indio_dev,
> @@ -499,20 +509,26 @@ static int ina2xx_write_raw(struct iio_dev *indio_dev,
> break;

Side note for follow up patches is this function would benefit a lot
from use of guard()

>
> case IIO_CHAN_INFO_INT_TIME:
> - if (chip->config->chip_id == ina226) {
> + switch (chip->config->chip_id) {
> + case ina226:
> + case ina236:
> if (chan->address == INA2XX_SHUNT_VOLTAGE)
> ret = ina226_set_int_time_vshunt(chip, val2,
> &tmp);
> else
> ret = ina226_set_int_time_vbus(chip, val2,
> &tmp);
> - } else {
> + break;
> + case ina219:
> if (chan->address == INA2XX_SHUNT_VOLTAGE)
> ret = ina219_set_int_time_vshunt(chip, val2,
> &tmp);
> else
> ret = ina219_set_int_time_vbus(chip, val2,
> &tmp);
> + break;
> + default:
> + ret = -EINVAL;
> }
> break;
>
> @@ -727,14 +743,20 @@ static int ina2xx_conversion_ready(struct iio_dev *indio_dev)
> * For now, we do an extra read of the MASK_ENABLE register (INA226)
> * resp. the BUS_VOLTAGE register (INA219).
> */
> - if (chip->config->chip_id == ina226) {
> + switch (chip->config->chip_id) {
> + case ina226:
> + case ina236:
> ret = regmap_read(chip->regmap,
> INA226_MASK_ENABLE, &alert);
> alert &= INA226_CVRF;
Not caused by your patch as it's existing code but it's really
ugly to check for an error only after modifying data that, if
there is an error would be uninitialized.

Can we clean that up whilst here by bringing duplicating the if (ret < 0)
and pulling it into these case blocks?

> - } else {
> + break;
> + case ina219:
> ret = regmap_read(chip->regmap,
> INA2XX_BUS_VOLTAGE, &alert);
> alert &= INA219_CNVR;
> + break;
> + default:
> + ret = -EINVAL;
This might as well return after moving if (ret < 0) into the other two.

(as noted above, i just did this whilst applying your patch).

> }
>
> if (ret < 0)