Re: [PATCH] (ina2xx) Drop bus_voltage_shift configuration

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

Hi,

On 3/2/26 07:26, Jonas Rebmann wrote:
> The INA219 has the lowest three bits of the bus voltage register
> zero-reserved and the bus_voltage_shift ina2xx_config field was
> introduced to accommodate for that.
> 
> The INA234 has four bits of the bus voltage, of the shunt voltage, and
> of the current registers zero-reserved but the latter two were
> implemented by choosing a 16x higher conversion constant instead of a
> separate field specifying a bit shift.
> 
> For consistency and simplicity, drop bus_voltage_shift and adapt the
> conversion constants for INA219 and INA234 accordingly, yielding the
> same measurement values.
> 

This isn't about simplicity, it is about correctness.

The datasheet for INA234 says for the lower 4 bits:

     Always returns 0. Remove these bits from the full result by doing a
     right arithmetic shift

which is what we should do for all chips with reserved bits instead of
assuming that the reserved bits always return 0.

Thanks,
Guenter

> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>   drivers/hwmon/ina2xx.c | 15 ++++-----------
>   1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> index 836e15a5a780..d7c894d7353c 100644
> --- a/drivers/hwmon/ina2xx.c
> +++ b/drivers/hwmon/ina2xx.c
> @@ -151,7 +151,6 @@ struct ina2xx_config {
>   	bool has_update_interval;
>   	int calibration_value;
>   	int shunt_div;
> -	int bus_voltage_shift;
>   	int bus_voltage_lsb;	/* uV */
>   	int power_lsb_factor;
>   };
> @@ -172,8 +171,7 @@ static const struct ina2xx_config ina2xx_config[] = {
>   		.config_default = INA219_CONFIG_DEFAULT,
>   		.calibration_value = 4096,
>   		.shunt_div = 100,
> -		.bus_voltage_shift = 3,
> -		.bus_voltage_lsb = 4000,
> +		.bus_voltage_lsb = 500,
>   		.power_lsb_factor = 20,
>   		.has_alerts = false,
>   		.has_ishunt = false,
> @@ -184,7 +182,6 @@ static const struct ina2xx_config ina2xx_config[] = {
>   		.config_default = INA226_CONFIG_DEFAULT,
>   		.calibration_value = 2048,
>   		.shunt_div = 400,
> -		.bus_voltage_shift = 0,
>   		.bus_voltage_lsb = 1250,
>   		.power_lsb_factor = 25,
>   		.has_alerts = true,
> @@ -196,8 +193,7 @@ static const struct ina2xx_config ina2xx_config[] = {
>   		.config_default = INA226_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,
> +		.bus_voltage_lsb = 1600,
>   		.power_lsb_factor = 32,
>   		.has_alerts = true,
>   		.has_ishunt = false,
> @@ -207,7 +203,6 @@ static const struct ina2xx_config ina2xx_config[] = {
>   	[ina260] = {
>   		.config_default = INA260_CONFIG_DEFAULT,
>   		.shunt_div = 400,
> -		.bus_voltage_shift = 0,
>   		.bus_voltage_lsb = 1250,
>   		.power_lsb_factor = 8,
>   		.has_alerts = true,
> @@ -219,7 +214,6 @@ static const struct ina2xx_config ina2xx_config[] = {
>   		.config_default = SY24655_CONFIG_DEFAULT,
>   		.calibration_value = 4096,
>   		.shunt_div = 400,
> -		.bus_voltage_shift = 0,
>   		.bus_voltage_lsb = 1250,
>   		.power_lsb_factor = 25,
>   		.has_alerts = true,
> @@ -281,8 +275,7 @@ static int ina2xx_get_value(struct ina2xx_data *data, u8 reg,
>   		val = DIV_ROUND_CLOSEST((s16)regval, data->config->shunt_div);
>   		break;
>   	case INA2XX_BUS_VOLTAGE:
> -		val = (regval >> data->config->bus_voltage_shift) *
> -		  data->config->bus_voltage_lsb;
> +		val = regval * data->config->bus_voltage_lsb;
>   		val = DIV_ROUND_CLOSEST(val, 1000);
>   		break;
>   	case INA2XX_POWER:
> @@ -387,7 +380,7 @@ static u16 ina226_alert_to_reg(struct ina2xx_data *data, int reg, long val)
>   		return clamp_val(val, 0, SHRT_MAX);
>   	case INA2XX_BUS_VOLTAGE:
>   		val = clamp_val(val, 0, 200000);
> -		val = (val * 1000) << data->config->bus_voltage_shift;
> +		val = val * 1000;
>   		val = DIV_ROUND_CLOSEST(val, data->config->bus_voltage_lsb);
>   		return clamp_val(val, 0, USHRT_MAX);
>   	case INA2XX_POWER:
> 
> ---
> base-commit: f08c5de5f61a117ba5326d3d5b86e884077da2d0
> change-id: 20260302-ina2xx-shift-89ed2c5d2e72
> 
> Best regards,
> --
> Jonas Rebmann <jre@pengutronix.de>
>