From: Guenter Roeck Date: Sun, 31 Aug 2025 00:38:51 +0000 (-0700) Subject: hwmon: (ina238) Rework and simplify temperature calculations X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=40a5da1ec101476dd6abb9cfa181f1b50dc24ad0;p=users%2Fhch%2Fmisc.git hwmon: (ina238) Rework and simplify temperature calculations The temperature register is 16 bit wide for all chips. The decimal point is at the same location (bit 7 = 1 degree C). That means we can use the resolution to calculate temperatures. Do that to simplify the code. There is only a single writeable temperature attribute, and it is very unlikely that the chips supported by this driver will ever require another one. That means checking for that attribute in the write function is unnecessary. Drop the check. Rename the write function from ina238_write_temp() to ina238_write_temp_max() to reflect that a single attribute is written. Also extend the accepted temperature value range to the range supported by the chip registers. Limiting the accepted value range to the temperature range supported by the chip would make it impossible to read an out-of-range limit from the chip and to write the same value back into it. This is undesirable, especially since the maximum temperature register does contain the maximum register value after a chip reset, not the temperature limit supported by the chip. 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 23195dead74f..e386a0f83fbb 100644 --- a/drivers/hwmon/ina238.c +++ b/drivers/hwmon/ina238.c @@ -103,10 +103,7 @@ #define INA238_SHUNT_VOLTAGE_LSB 5 /* 5 uV/lsb */ #define INA238_BUS_VOLTAGE_LSB 3125 /* 3.125 mV/lsb */ -#define INA238_DIE_TEMP_LSB 1250000 /* 125.0000 mC/lsb */ #define SQ52206_BUS_VOLTAGE_LSB 3750 /* 3.75 mV/lsb */ -#define SQ52206_DIE_TEMP_LSB 78125 /* 7.8125 mC/lsb */ -#define INA228_DIE_TEMP_LSB 78125 /* 7.8125 mC/lsb */ static const struct regmap_config ina238_regmap_config = { .max_register = INA238_REGISTERS, @@ -120,11 +117,10 @@ struct ina238_config { bool has_20bit_voltage_current; /* vshunt, vbus and current are 20-bit fields */ bool has_power_highest; /* chip detection power peak */ bool has_energy; /* chip detection energy */ - u8 temp_shift; /* fixed parameters for temp calculate */ + u8 temp_resolution; /* temperature register resolution in bit */ u32 power_calculate_factor; /* fixed parameters for power calculate */ u16 config_default; /* Power-on default state */ int bus_voltage_lsb; /* use for temperature calculate, uV/lsb */ - int temp_lsb; /* use for temperature calculate */ }; struct ina238_data { @@ -141,41 +137,37 @@ static const struct ina238_config ina238_config[] = { .has_20bit_voltage_current = false, .has_energy = false, .has_power_highest = false, - .temp_shift = 4, .power_calculate_factor = 20, .config_default = INA238_CONFIG_DEFAULT, .bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB, - .temp_lsb = INA238_DIE_TEMP_LSB, + .temp_resolution = 12, }, [ina237] = { .has_20bit_voltage_current = false, .has_energy = false, .has_power_highest = false, - .temp_shift = 4, .power_calculate_factor = 20, .config_default = INA238_CONFIG_DEFAULT, .bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB, - .temp_lsb = INA238_DIE_TEMP_LSB, + .temp_resolution = 12, }, [sq52206] = { .has_20bit_voltage_current = false, .has_energy = true, .has_power_highest = true, - .temp_shift = 0, .power_calculate_factor = 24, .config_default = SQ52206_CONFIG_DEFAULT, .bus_voltage_lsb = SQ52206_BUS_VOLTAGE_LSB, - .temp_lsb = SQ52206_DIE_TEMP_LSB, + .temp_resolution = 16, }, [ina228] = { .has_20bit_voltage_current = true, .has_energy = true, .has_power_highest = false, - .temp_shift = 0, .power_calculate_factor = 20, .config_default = INA238_CONFIG_DEFAULT, .bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB, - .temp_lsb = INA228_DIE_TEMP_LSB, + .temp_resolution = 16, }, }; @@ -522,6 +514,11 @@ static int ina238_write_power_max(struct device *dev, long val) return regmap_write(data->regmap, INA238_POWER_LIMIT, regval); } +static int ina238_temp_from_reg(s16 regval, u8 resolution) +{ + return ((regval >> (16 - resolution)) * 1000) >> (resolution - 9); +} + static int ina238_read_temp(struct device *dev, u32 attr, long *val) { struct ina238_data *data = dev_get_drvdata(dev); @@ -533,17 +530,14 @@ static int ina238_read_temp(struct device *dev, u32 attr, long *val) err = regmap_read(data->regmap, INA238_DIE_TEMP, ®val); if (err) return err; - /* Signed, result in mC */ - *val = div_s64(((s64)((s16)regval) >> data->config->temp_shift) * - (s64)data->config->temp_lsb, 10000); + *val = ina238_temp_from_reg(regval, data->config->temp_resolution); break; case hwmon_temp_max: err = regmap_read(data->regmap, INA238_TEMP_LIMIT, ®val); if (err) return err; /* Signed, result in mC */ - *val = div_s64(((s64)((s16)regval) >> data->config->temp_shift) * - (s64)data->config->temp_lsb, 10000); + *val = ina238_temp_from_reg(regval, data->config->temp_resolution); break; case hwmon_temp_max_alarm: err = regmap_read(data->regmap, INA238_DIAG_ALERT, ®val); @@ -559,19 +553,21 @@ static int ina238_read_temp(struct device *dev, u32 attr, long *val) return 0; } -static int ina238_write_temp(struct device *dev, u32 attr, long val) +static u16 ina238_temp_to_reg(long val, u8 resolution) { - struct ina238_data *data = dev_get_drvdata(dev); - int regval; + int fraction = 1000 - DIV_ROUND_CLOSEST(1000, BIT(resolution - 9)); - if (attr != hwmon_temp_max) - return -EOPNOTSUPP; + val = clamp_val(val, -255000 - fraction, 255000 + fraction); + + return (DIV_ROUND_CLOSEST(val << (resolution - 9), 1000) << (16 - resolution)) & 0xffff; +} - /* Signed */ - val = clamp_val(val, -40000, 125000); - regval = div_s64(val * 10000, data->config->temp_lsb) << data->config->temp_shift; - regval = clamp_val(regval, S16_MIN, S16_MAX) & (0xffff << data->config->temp_shift); +static int ina238_write_temp_max(struct device *dev, long val) +{ + struct ina238_data *data = dev_get_drvdata(dev); + int regval; + regval = ina238_temp_to_reg(val, data->config->temp_resolution); return regmap_write(data->regmap, INA238_TEMP_LIMIT, regval); } @@ -628,7 +624,7 @@ static int ina238_write(struct device *dev, enum hwmon_sensor_types type, err = ina238_write_power_max(dev, val); break; case hwmon_temp: - err = ina238_write_temp(dev, attr, val); + err = ina238_write_temp_max(dev, val); break; default: err = -EOPNOTSUPP;