Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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 <chris.packham@alliedtelesis.co.nz>
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz> # INA780
Signed-off-by: Guenter Roeck <linux@roeck-us.net>

+53 -108
+53 -108
drivers/hwmon/ina238.c
··· 101 101 #define INA238_CALIBRATION_VALUE 16384 102 102 #define INA238_FIXED_SHUNT 20000 103 103 104 - #define INA238_SHUNT_VOLTAGE_LSB 5 /* 5 uV/lsb */ 105 - #define INA238_BUS_VOLTAGE_LSB 3125 /* 3.125 mV/lsb */ 106 - #define SQ52206_BUS_VOLTAGE_LSB 3750 /* 3.75 mV/lsb */ 104 + #define INA238_SHUNT_VOLTAGE_LSB 5000 /* 5 uV/lsb, in nV */ 105 + #define INA238_BUS_VOLTAGE_LSB 3125000 /* 3.125 mV/lsb, in nV */ 106 + #define SQ52206_BUS_VOLTAGE_LSB 3750000 /* 3.75 mV/lsb, in nV */ 107 + 108 + #define NUNIT_PER_MUNIT 1000000 /* n[AV] -> m[AV] */ 107 109 108 110 static const struct regmap_config ina238_regmap_config = { 109 111 .max_register = INA238_REGISTERS, ··· 120 118 bool has_power_highest; /* chip detection power peak */ 121 119 bool has_energy; /* chip detection energy */ 122 120 u8 temp_resolution; /* temperature register resolution in bit */ 123 - u32 power_calculate_factor; /* fixed parameter for power calculation, from datasheet */ 124 121 u16 config_default; /* Power-on default state */ 125 - int bus_voltage_lsb; /* use for temperature calculate, uV/lsb */ 122 + u32 power_calculate_factor; /* fixed parameter for power calculation, from datasheet */ 123 + u32 bus_voltage_lsb; /* bus voltage LSB, in nV */ 126 124 int current_lsb; /* current LSB, in uA */ 127 125 }; 128 126 ··· 133 131 struct regmap *regmap; 134 132 u32 rshunt; 135 133 int gain; 134 + u32 voltage_lsb[2]; /* shunt, bus voltage LSB, in nV */ 136 135 int current_lsb; /* current LSB, in uA */ 137 136 int power_lsb; /* power LSB, in uW */ 138 137 int energy_lsb; /* energy LSB, in uJ */ ··· 229 226 return 0; 230 227 } 231 228 232 - static int ina228_read_shunt_voltage(struct device *dev, u32 attr, int channel, 233 - long *val) 229 + static int ina228_read_voltage(struct ina238_data *data, int channel, long *val) 234 230 { 235 - struct ina238_data *data = dev_get_drvdata(dev); 236 - int regval; 237 - int err; 231 + int reg = channel ? INA238_BUS_VOLTAGE : INA238_SHUNT_VOLTAGE; 232 + u32 lsb = data->voltage_lsb[channel]; 233 + u32 factor = NUNIT_PER_MUNIT; 234 + int err, regval; 238 235 239 - err = ina238_read_field_s20(data->client, INA238_SHUNT_VOLTAGE, &regval); 240 - if (err) 241 - return err; 236 + if (data->config->has_20bit_voltage_current) { 237 + err = ina238_read_field_s20(data->client, reg, &regval); 238 + if (err) 239 + return err; 240 + /* Adjust accuracy: LSB in units of 500 pV */ 241 + lsb /= 8; 242 + factor *= 2; 243 + } else { 244 + err = regmap_read(data->regmap, reg, &regval); 245 + if (err) 246 + return err; 247 + regval = (s16)regval; 248 + } 242 249 243 - /* 244 - * gain of 1 -> LSB / 4 245 - * This field has 16 bit on ina238. ina228 adds another 4 bits of 246 - * precision. ina238 conversion factors can still be applied when 247 - * dividing by 16. 248 - */ 249 - *val = (regval * INA238_SHUNT_VOLTAGE_LSB) * data->gain / (1000 * 4) / 16; 250 - return 0; 251 - } 252 - 253 - static int ina228_read_bus_voltage(struct device *dev, u32 attr, int channel, 254 - long *val) 255 - { 256 - struct ina238_data *data = dev_get_drvdata(dev); 257 - int regval; 258 - int err; 259 - 260 - err = ina238_read_field_s20(data->client, INA238_BUS_VOLTAGE, &regval); 261 - if (err) 262 - return err; 263 - 264 - /* 265 - * gain of 1 -> LSB / 4 266 - * This field has 16 bit on ina238. ina228 adds another 4 bits of 267 - * precision. ina238 conversion factors can still be applied when 268 - * dividing by 16. 269 - */ 270 - *val = (regval * data->config->bus_voltage_lsb) / 1000 / 16; 250 + *val = DIV_S64_ROUND_CLOSEST((s64)regval * lsb, factor); 271 251 return 0; 272 252 } 273 253 ··· 258 272 long *val) 259 273 { 260 274 struct ina238_data *data = dev_get_drvdata(dev); 261 - int reg, mask; 275 + int reg, mask = 0; 262 276 int regval; 263 277 int err; 278 + 279 + if (attr == hwmon_in_input) 280 + return ina228_read_voltage(data, channel, val); 264 281 265 282 switch (channel) { 266 283 case 0: 267 284 switch (attr) { 268 - case hwmon_in_input: 269 - if (data->config->has_20bit_voltage_current) 270 - return ina228_read_shunt_voltage(dev, attr, channel, val); 271 - reg = INA238_SHUNT_VOLTAGE; 272 - break; 273 285 case hwmon_in_max: 274 286 reg = INA238_SHUNT_OVER_VOLTAGE; 275 287 break; ··· 288 304 break; 289 305 case 1: 290 306 switch (attr) { 291 - case hwmon_in_input: 292 - if (data->config->has_20bit_voltage_current) 293 - return ina228_read_bus_voltage(dev, attr, channel, val); 294 - reg = INA238_BUS_VOLTAGE; 295 - break; 296 307 case hwmon_in_max: 297 308 reg = INA238_BUS_OVER_VOLTAGE; 298 309 break; ··· 314 335 if (err < 0) 315 336 return err; 316 337 317 - switch (attr) { 318 - case hwmon_in_input: 319 - case hwmon_in_max: 320 - case hwmon_in_min: 321 - /* signed register, value in mV */ 322 - regval = (s16)regval; 323 - if (channel == 0) 324 - /* gain of 1 -> LSB / 4 */ 325 - *val = (regval * INA238_SHUNT_VOLTAGE_LSB) * 326 - data->gain / (1000 * 4); 327 - else 328 - *val = (regval * data->config->bus_voltage_lsb) / 1000; 329 - break; 330 - case hwmon_in_max_alarm: 331 - case hwmon_in_min_alarm: 338 + if (mask) 332 339 *val = !!(regval & mask); 333 - break; 334 - } 340 + else 341 + *val = DIV_S64_ROUND_CLOSEST((s64)(s16)regval * data->voltage_lsb[channel], 342 + NUNIT_PER_MUNIT); 335 343 336 344 return 0; 337 345 } 338 346 339 - static int ina238_write_in(struct device *dev, u32 attr, int channel, 340 - long val) 347 + static int ina238_write_in(struct device *dev, u32 attr, int channel, long val) 341 348 { 342 349 struct ina238_data *data = dev_get_drvdata(dev); 350 + static const int low_limits[2] = {-164, 0}; 351 + static const int high_limits[2] = {164, 150000}; 352 + static const u8 low_regs[2] = {INA238_SHUNT_UNDER_VOLTAGE, INA238_BUS_UNDER_VOLTAGE}; 353 + static const u8 high_regs[2] = {INA238_SHUNT_OVER_VOLTAGE, INA238_BUS_OVER_VOLTAGE}; 343 354 int regval; 344 355 345 - if (attr != hwmon_in_max && attr != hwmon_in_min) 346 - return -EOPNOTSUPP; 356 + /* Initial clamp to avoid overflows */ 357 + val = clamp_val(val, low_limits[channel], high_limits[channel]); 358 + val = DIV_S64_ROUND_CLOSEST((s64)val * NUNIT_PER_MUNIT, data->voltage_lsb[channel]); 359 + /* Final clamp to register limits */ 360 + regval = clamp_val(val, S16_MIN, S16_MAX) & 0xffff; 347 361 348 - /* convert decimal to register value */ 349 - switch (channel) { 350 - case 0: 351 - /* signed value, clamp to max range +/-163 mV */ 352 - regval = clamp_val(val, -163, 163); 353 - regval = (regval * 1000 * 4) / 354 - (INA238_SHUNT_VOLTAGE_LSB * data->gain); 355 - regval = clamp_val(regval, S16_MIN, S16_MAX) & 0xffff; 356 - 357 - switch (attr) { 358 - case hwmon_in_max: 359 - return regmap_write(data->regmap, 360 - INA238_SHUNT_OVER_VOLTAGE, regval); 361 - case hwmon_in_min: 362 - return regmap_write(data->regmap, 363 - INA238_SHUNT_UNDER_VOLTAGE, regval); 364 - default: 365 - return -EOPNOTSUPP; 366 - } 367 - case 1: 368 - /* signed value, positive values only. Clamp to max 102.396 V */ 369 - regval = clamp_val(val, 0, 102396); 370 - regval = (regval * 1000) / data->config->bus_voltage_lsb; 371 - regval = clamp_val(regval, 0, S16_MAX); 372 - 373 - switch (attr) { 374 - case hwmon_in_max: 375 - return regmap_write(data->regmap, 376 - INA238_BUS_OVER_VOLTAGE, regval); 377 - case hwmon_in_min: 378 - return regmap_write(data->regmap, 379 - INA238_BUS_UNDER_VOLTAGE, regval); 380 - default: 381 - return -EOPNOTSUPP; 382 - } 362 + switch (attr) { 363 + case hwmon_in_min: 364 + return regmap_write(data->regmap, low_regs[channel], regval); 365 + case hwmon_in_max: 366 + return regmap_write(data->regmap, high_regs[channel], regval); 383 367 default: 384 368 return -EOPNOTSUPP; 385 369 } ··· 753 811 dev_err(dev, "error configuring the device: %d\n", ret); 754 812 return -ENODEV; 755 813 } 814 + 815 + data->voltage_lsb[0] = INA238_SHUNT_VOLTAGE_LSB * data->gain / 4; 816 + data->voltage_lsb[1] = data->config->bus_voltage_lsb; 756 817 757 818 if (data->config->current_lsb) 758 819 data->current_lsb = data->config->current_lsb;