From: Guenter Roeck Date: Sun, 31 Aug 2025 04:48:29 +0000 (-0700) Subject: hwmon: (ina238) Simplify voltage register accesses X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=7e420b6a274206f339fe3617943ba1ef9dd1fcb0;p=users%2Fhch%2Fmisc.git hwmon: (ina238) Simplify voltage register accesses Calculate voltage LSB values in the probe function and use throughout the code. Use a single function to read all voltages, independently of the register width. Use the pre-calculated LSB values to convert register values to voltages and do not rely on runtime chip specific code. Use ROUND_CLOSEST functions instead of divide operations to reduce rounding errors. Reviewed-by: Chris Packham Tested-by: Chris Packham # INA780 Signed-off-by: Guenter Roeck --- diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c index 316a7dc720f5..ae27ae2582f2 100644 --- a/drivers/hwmon/ina238.c +++ b/drivers/hwmon/ina238.c @@ -101,9 +101,11 @@ #define INA238_CALIBRATION_VALUE 16384 #define INA238_FIXED_SHUNT 20000 -#define INA238_SHUNT_VOLTAGE_LSB 5 /* 5 uV/lsb */ -#define INA238_BUS_VOLTAGE_LSB 3125 /* 3.125 mV/lsb */ -#define SQ52206_BUS_VOLTAGE_LSB 3750 /* 3.75 mV/lsb */ +#define INA238_SHUNT_VOLTAGE_LSB 5000 /* 5 uV/lsb, in nV */ +#define INA238_BUS_VOLTAGE_LSB 3125000 /* 3.125 mV/lsb, in nV */ +#define SQ52206_BUS_VOLTAGE_LSB 3750000 /* 3.75 mV/lsb, in nV */ + +#define NUNIT_PER_MUNIT 1000000 /* n[AV] -> m[AV] */ static const struct regmap_config ina238_regmap_config = { .max_register = INA238_REGISTERS, @@ -118,9 +120,9 @@ struct ina238_config { bool has_power_highest; /* chip detection power peak */ bool has_energy; /* chip detection energy */ u8 temp_resolution; /* temperature register resolution in bit */ - u32 power_calculate_factor; /* fixed parameter for power calculation, from datasheet */ u16 config_default; /* Power-on default state */ - int bus_voltage_lsb; /* use for temperature calculate, uV/lsb */ + u32 power_calculate_factor; /* fixed parameter for power calculation, from datasheet */ + u32 bus_voltage_lsb; /* bus voltage LSB, in nV */ int current_lsb; /* current LSB, in uA */ }; @@ -131,6 +133,7 @@ struct ina238_data { struct regmap *regmap; u32 rshunt; int gain; + u32 voltage_lsb[2]; /* shunt, bus voltage LSB, in nV */ int current_lsb; /* current LSB, in uA */ int power_lsb; /* power LSB, in uW */ int energy_lsb; /* energy LSB, in uJ */ @@ -226,45 +229,28 @@ static int ina238_read_field_s20(const struct i2c_client *client, u8 reg, s32 *v return 0; } -static int ina228_read_shunt_voltage(struct device *dev, u32 attr, int channel, - long *val) -{ - struct ina238_data *data = dev_get_drvdata(dev); - int regval; - int err; - - err = ina238_read_field_s20(data->client, INA238_SHUNT_VOLTAGE, ®val); - if (err) - return err; - - /* - * gain of 1 -> LSB / 4 - * This field has 16 bit on ina238. ina228 adds another 4 bits of - * precision. ina238 conversion factors can still be applied when - * dividing by 16. - */ - *val = (regval * INA238_SHUNT_VOLTAGE_LSB) * data->gain / (1000 * 4) / 16; - return 0; -} - -static int ina228_read_bus_voltage(struct device *dev, u32 attr, int channel, - long *val) +static int ina228_read_voltage(struct ina238_data *data, int channel, long *val) { - struct ina238_data *data = dev_get_drvdata(dev); - int regval; - int err; + int reg = channel ? INA238_BUS_VOLTAGE : INA238_SHUNT_VOLTAGE; + u32 lsb = data->voltage_lsb[channel]; + u32 factor = NUNIT_PER_MUNIT; + int err, regval; - err = ina238_read_field_s20(data->client, INA238_BUS_VOLTAGE, ®val); - if (err) - return err; + if (data->config->has_20bit_voltage_current) { + err = ina238_read_field_s20(data->client, reg, ®val); + if (err) + return err; + /* Adjust accuracy: LSB in units of 500 pV */ + lsb /= 8; + factor *= 2; + } else { + err = regmap_read(data->regmap, reg, ®val); + if (err) + return err; + regval = (s16)regval; + } - /* - * gain of 1 -> LSB / 4 - * This field has 16 bit on ina238. ina228 adds another 4 bits of - * precision. ina238 conversion factors can still be applied when - * dividing by 16. - */ - *val = (regval * data->config->bus_voltage_lsb) / 1000 / 16; + *val = DIV_S64_ROUND_CLOSEST((s64)regval * lsb, factor); return 0; } @@ -272,18 +258,16 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel, long *val) { struct ina238_data *data = dev_get_drvdata(dev); - int reg, mask; + int reg, mask = 0; int regval; int err; + if (attr == hwmon_in_input) + return ina228_read_voltage(data, channel, val); + switch (channel) { case 0: switch (attr) { - case hwmon_in_input: - if (data->config->has_20bit_voltage_current) - return ina228_read_shunt_voltage(dev, attr, channel, val); - reg = INA238_SHUNT_VOLTAGE; - break; case hwmon_in_max: reg = INA238_SHUNT_OVER_VOLTAGE; break; @@ -304,11 +288,6 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel, break; case 1: switch (attr) { - case hwmon_in_input: - if (data->config->has_20bit_voltage_current) - return ina228_read_bus_voltage(dev, attr, channel, val); - reg = INA238_BUS_VOLTAGE; - break; case hwmon_in_max: reg = INA238_BUS_OVER_VOLTAGE; break; @@ -335,72 +314,35 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel, if (err < 0) return err; - switch (attr) { - case hwmon_in_input: - case hwmon_in_max: - case hwmon_in_min: - /* signed register, value in mV */ - regval = (s16)regval; - if (channel == 0) - /* gain of 1 -> LSB / 4 */ - *val = (regval * INA238_SHUNT_VOLTAGE_LSB) * - data->gain / (1000 * 4); - else - *val = (regval * data->config->bus_voltage_lsb) / 1000; - break; - case hwmon_in_max_alarm: - case hwmon_in_min_alarm: + if (mask) *val = !!(regval & mask); - break; - } + else + *val = DIV_S64_ROUND_CLOSEST((s64)(s16)regval * data->voltage_lsb[channel], + NUNIT_PER_MUNIT); return 0; } -static int ina238_write_in(struct device *dev, u32 attr, int channel, - long val) +static int ina238_write_in(struct device *dev, u32 attr, int channel, long val) { struct ina238_data *data = dev_get_drvdata(dev); + static const int low_limits[2] = {-164, 0}; + static const int high_limits[2] = {164, 150000}; + static const u8 low_regs[2] = {INA238_SHUNT_UNDER_VOLTAGE, INA238_BUS_UNDER_VOLTAGE}; + static const u8 high_regs[2] = {INA238_SHUNT_OVER_VOLTAGE, INA238_BUS_OVER_VOLTAGE}; int regval; - if (attr != hwmon_in_max && attr != hwmon_in_min) - return -EOPNOTSUPP; - - /* convert decimal to register value */ - switch (channel) { - case 0: - /* signed value, clamp to max range +/-163 mV */ - regval = clamp_val(val, -163, 163); - regval = (regval * 1000 * 4) / - (INA238_SHUNT_VOLTAGE_LSB * data->gain); - regval = clamp_val(regval, S16_MIN, S16_MAX) & 0xffff; - - switch (attr) { - case hwmon_in_max: - return regmap_write(data->regmap, - INA238_SHUNT_OVER_VOLTAGE, regval); - case hwmon_in_min: - return regmap_write(data->regmap, - INA238_SHUNT_UNDER_VOLTAGE, regval); - default: - return -EOPNOTSUPP; - } - case 1: - /* signed value, positive values only. Clamp to max 102.396 V */ - regval = clamp_val(val, 0, 102396); - regval = (regval * 1000) / data->config->bus_voltage_lsb; - regval = clamp_val(regval, 0, S16_MAX); + /* Initial clamp to avoid overflows */ + val = clamp_val(val, low_limits[channel], high_limits[channel]); + val = DIV_S64_ROUND_CLOSEST((s64)val * NUNIT_PER_MUNIT, data->voltage_lsb[channel]); + /* Final clamp to register limits */ + regval = clamp_val(val, S16_MIN, S16_MAX) & 0xffff; - switch (attr) { - case hwmon_in_max: - return regmap_write(data->regmap, - INA238_BUS_OVER_VOLTAGE, regval); - case hwmon_in_min: - return regmap_write(data->regmap, - INA238_BUS_UNDER_VOLTAGE, regval); - default: - return -EOPNOTSUPP; - } + switch (attr) { + case hwmon_in_min: + return regmap_write(data->regmap, low_regs[channel], regval); + case hwmon_in_max: + return regmap_write(data->regmap, high_regs[channel], regval); default: return -EOPNOTSUPP; } @@ -812,6 +754,9 @@ static int ina238_probe(struct i2c_client *client) return -ENODEV; } + data->voltage_lsb[0] = INA238_SHUNT_VOLTAGE_LSB * data->gain / 4; + data->voltage_lsb[1] = data->config->bus_voltage_lsb; + if (data->config->current_lsb) data->current_lsb = data->config->current_lsb; else