| Message ID | 20210316175503.1003051-1-kubernat@cesnet.cz (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/5] hwmon: (max31790) Rework to use regmap | expand |
Hi Guenther, I'm posting v2 of my series on max31790. These are the changes from v1: 1) The driver uses regmap caching instead of local caching. 2) I got rid of the "refactor" patch and also the "pulses" patch. 3) I got rid of unnecessary whitespace changes. 4) fan*_input now returns -ENODATA when fan*_enable is 0. 5) Changed the argument of bits_for_speed_range to long. Vaclav
Hi "Václav,
url: https://github.com/0day-ci/linux/commits/V-clav-Kubern-t/hwmon-max31790-Rework-to-use-regmap/20210317-015931
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: x86_64-randconfig-m001-20210316 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/hwmon/max31790.c:263 max31790_fan_is_visible() warn: impossible condition '(fan_config < 0) => (0-255 < 0)'
drivers/hwmon/max31790.c:337 max31790_write_pwm() warn: impossible condition '(fan_config < 0) => (0-255 < 0)'
drivers/hwmon/max31790.c:372 max31790_pwm_is_visible() warn: impossible condition '(fan_config < 0) => (0-255 < 0)'
vim +263 drivers/hwmon/max31790.c
54187ff9d766b2 Guenter Roeck 2016-07-01 257 static umode_t max31790_fan_is_visible(const void *_data, u32 attr, int channel)
195a4b4298a795 Il Han 2015-08-30 258 {
54187ff9d766b2 Guenter Roeck 2016-07-01 259 const struct max31790_data *data = _data;
2c8602cfaeab63 Václav Kubernát 2021-03-16 260 struct regmap *regmap = data->regmap;
2c8602cfaeab63 Václav Kubernát 2021-03-16 261 u8 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL));
2c8602cfaeab63 Václav Kubernát 2021-03-16 262
2c8602cfaeab63 Václav Kubernát 2021-03-16 @263 if (fan_config < 0)
^^^^^^^^^^^^^^
A u8 can't be negative.
2c8602cfaeab63 Václav Kubernát 2021-03-16 264 return 0;
54187ff9d766b2 Guenter Roeck 2016-07-01 265
54187ff9d766b2 Guenter Roeck 2016-07-01 266 switch (attr) {
54187ff9d766b2 Guenter Roeck 2016-07-01 267 case hwmon_fan_input:
54187ff9d766b2 Guenter Roeck 2016-07-01 268 case hwmon_fan_fault:
54187ff9d766b2 Guenter Roeck 2016-07-01 269 if (channel < NR_CHANNEL ||
54187ff9d766b2 Guenter Roeck 2016-07-01 270 (fan_config & MAX31790_FAN_CFG_TACH_INPUT))
dc8dbb4d7672b7 Guenter Roeck 2018-12-10 271 return 0444;
54187ff9d766b2 Guenter Roeck 2016-07-01 272 return 0;
54187ff9d766b2 Guenter Roeck 2016-07-01 273 case hwmon_fan_target:
54187ff9d766b2 Guenter Roeck 2016-07-01 274 if (channel < NR_CHANNEL &&
54187ff9d766b2 Guenter Roeck 2016-07-01 275 !(fan_config & MAX31790_FAN_CFG_TACH_INPUT))
dc8dbb4d7672b7 Guenter Roeck 2018-12-10 276 return 0644;
54187ff9d766b2 Guenter Roeck 2016-07-01 277 return 0;
54187ff9d766b2 Guenter Roeck 2016-07-01 278 default:
54187ff9d766b2 Guenter Roeck 2016-07-01 279 return 0;
195a4b4298a795 Il Han 2015-08-30 280 }
195a4b4298a795 Il Han 2015-08-30 281 }
195a4b4298a795 Il Han 2015-08-30 282
54187ff9d766b2 Guenter Roeck 2016-07-01 283 static int max31790_read_pwm(struct device *dev, u32 attr, int channel,
54187ff9d766b2 Guenter Roeck 2016-07-01 284 long *val)
195a4b4298a795 Il Han 2015-08-30 285 {
2c8602cfaeab63 Václav Kubernát 2021-03-16 286 struct max31790_data *data = dev_get_drvdata(dev);
2c8602cfaeab63 Václav Kubernát 2021-03-16 287 struct regmap *regmap = data->regmap;
2c8602cfaeab63 Václav Kubernát 2021-03-16 288 int read;
195a4b4298a795 Il Han 2015-08-30 289
195a4b4298a795 Il Han 2015-08-30 290 if (IS_ERR(data))
195a4b4298a795 Il Han 2015-08-30 291 return PTR_ERR(data);
195a4b4298a795 Il Han 2015-08-30 292
54187ff9d766b2 Guenter Roeck 2016-07-01 293 switch (attr) {
54187ff9d766b2 Guenter Roeck 2016-07-01 294 case hwmon_pwm_input:
2c8602cfaeab63 Václav Kubernát 2021-03-16 295 read = read_reg_word(regmap, MAX31790_REG_PWMOUT(channel));
2c8602cfaeab63 Václav Kubernát 2021-03-16 296 if (read < 0)
2c8602cfaeab63 Václav Kubernát 2021-03-16 297 return read;
2c8602cfaeab63 Václav Kubernát 2021-03-16 298
2c8602cfaeab63 Václav Kubernát 2021-03-16 299 *val = read >> 8;
54187ff9d766b2 Guenter Roeck 2016-07-01 300 return 0;
54187ff9d766b2 Guenter Roeck 2016-07-01 301 case hwmon_pwm_enable:
2c8602cfaeab63 Václav Kubernát 2021-03-16 302 read = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel));
2c8602cfaeab63 Václav Kubernát 2021-03-16 303 if (read < 0)
2c8602cfaeab63 Václav Kubernát 2021-03-16 304 return read;
2c8602cfaeab63 Václav Kubernát 2021-03-16 305
2c8602cfaeab63 Václav Kubernát 2021-03-16 306 if (read & MAX31790_FAN_CFG_RPM_MODE)
54187ff9d766b2 Guenter Roeck 2016-07-01 307 *val = 2;
2c8602cfaeab63 Václav Kubernát 2021-03-16 308 else if (read & MAX31790_FAN_CFG_TACH_INPUT_EN)
54187ff9d766b2 Guenter Roeck 2016-07-01 309 *val = 1;
195a4b4298a795 Il Han 2015-08-30 310 else
54187ff9d766b2 Guenter Roeck 2016-07-01 311 *val = 0;
54187ff9d766b2 Guenter Roeck 2016-07-01 312 return 0;
54187ff9d766b2 Guenter Roeck 2016-07-01 313 default:
54187ff9d766b2 Guenter Roeck 2016-07-01 314 return -EOPNOTSUPP;
54187ff9d766b2 Guenter Roeck 2016-07-01 315 }
195a4b4298a795 Il Han 2015-08-30 316 }
195a4b4298a795 Il Han 2015-08-30 317
54187ff9d766b2 Guenter Roeck 2016-07-01 318 static int max31790_write_pwm(struct device *dev, u32 attr, int channel,
54187ff9d766b2 Guenter Roeck 2016-07-01 319 long val)
195a4b4298a795 Il Han 2015-08-30 320 {
195a4b4298a795 Il Han 2015-08-30 321 struct max31790_data *data = dev_get_drvdata(dev);
2c8602cfaeab63 Václav Kubernát 2021-03-16 322 struct regmap *regmap = data->regmap;
54187ff9d766b2 Guenter Roeck 2016-07-01 323 u8 fan_config;
54187ff9d766b2 Guenter Roeck 2016-07-01 324 int err = 0;
195a4b4298a795 Il Han 2015-08-30 325
54187ff9d766b2 Guenter Roeck 2016-07-01 326 switch (attr) {
54187ff9d766b2 Guenter Roeck 2016-07-01 327 case hwmon_pwm_input:
54187ff9d766b2 Guenter Roeck 2016-07-01 328 if (val < 0 || val > 255) {
54187ff9d766b2 Guenter Roeck 2016-07-01 329 err = -EINVAL;
54187ff9d766b2 Guenter Roeck 2016-07-01 330 break;
54187ff9d766b2 Guenter Roeck 2016-07-01 331 }
2c8602cfaeab63 Václav Kubernát 2021-03-16 332 err = write_reg_word(regmap, MAX31790_REG_PWMOUT(channel), val << 8);
195a4b4298a795 Il Han 2015-08-30 333 break;
54187ff9d766b2 Guenter Roeck 2016-07-01 334 case hwmon_pwm_enable:
2c8602cfaeab63 Václav Kubernát 2021-03-16 335 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL));
2c8602cfaeab63 Václav Kubernát 2021-03-16 336
2c8602cfaeab63 Václav Kubernát 2021-03-16 @337 if (fan_config < 0)
2c8602cfaeab63 Václav Kubernát 2021-03-16 338 return fan_config;
2c8602cfaeab63 Václav Kubernát 2021-03-16 339
54187ff9d766b2 Guenter Roeck 2016-07-01 340 if (val == 0) {
54187ff9d766b2 Guenter Roeck 2016-07-01 341 fan_config &= ~(MAX31790_FAN_CFG_TACH_INPUT_EN |
54187ff9d766b2 Guenter Roeck 2016-07-01 342 MAX31790_FAN_CFG_RPM_MODE);
54187ff9d766b2 Guenter Roeck 2016-07-01 343 } else if (val == 1) {
54187ff9d766b2 Guenter Roeck 2016-07-01 344 fan_config = (fan_config |
54187ff9d766b2 Guenter Roeck 2016-07-01 345 MAX31790_FAN_CFG_TACH_INPUT_EN) &
54187ff9d766b2 Guenter Roeck 2016-07-01 346 ~MAX31790_FAN_CFG_RPM_MODE;
54187ff9d766b2 Guenter Roeck 2016-07-01 347 } else if (val == 2) {
54187ff9d766b2 Guenter Roeck 2016-07-01 348 fan_config |= MAX31790_FAN_CFG_TACH_INPUT_EN |
54187ff9d766b2 Guenter Roeck 2016-07-01 349 MAX31790_FAN_CFG_RPM_MODE;
54187ff9d766b2 Guenter Roeck 2016-07-01 350 } else {
54187ff9d766b2 Guenter Roeck 2016-07-01 351 err = -EINVAL;
195a4b4298a795 Il Han 2015-08-30 352 break;
54187ff9d766b2 Guenter Roeck 2016-07-01 353 }
2c8602cfaeab63 Václav Kubernát 2021-03-16 354 err = regmap_write(regmap,
54187ff9d766b2 Guenter Roeck 2016-07-01 355 MAX31790_REG_FAN_CONFIG(channel),
54187ff9d766b2 Guenter Roeck 2016-07-01 356 fan_config);
195a4b4298a795 Il Han 2015-08-30 357 break;
195a4b4298a795 Il Han 2015-08-30 358 default:
54187ff9d766b2 Guenter Roeck 2016-07-01 359 err = -EOPNOTSUPP;
54187ff9d766b2 Guenter Roeck 2016-07-01 360 break;
195a4b4298a795 Il Han 2015-08-30 361 }
195a4b4298a795 Il Han 2015-08-30 362
195a4b4298a795 Il Han 2015-08-30 363 return err;
195a4b4298a795 Il Han 2015-08-30 364 }
195a4b4298a795 Il Han 2015-08-30 365
54187ff9d766b2 Guenter Roeck 2016-07-01 366 static umode_t max31790_pwm_is_visible(const void *_data, u32 attr, int channel)
195a4b4298a795 Il Han 2015-08-30 367 {
54187ff9d766b2 Guenter Roeck 2016-07-01 368 const struct max31790_data *data = _data;
2c8602cfaeab63 Václav Kubernát 2021-03-16 369 struct regmap *regmap = data->regmap;
2c8602cfaeab63 Václav Kubernát 2021-03-16 370 u8 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL));
2c8602cfaeab63 Václav Kubernát 2021-03-16 371
2c8602cfaeab63 Václav Kubernát 2021-03-16 @372 if (fan_config < 0)
2c8602cfaeab63 Václav Kubernát 2021-03-16 373 return 0;
54187ff9d766b2 Guenter Roeck 2016-07-01 374
54187ff9d766b2 Guenter Roeck 2016-07-01 375 switch (attr) {
54187ff9d766b2 Guenter Roeck 2016-07-01 376 case hwmon_pwm_input:
54187ff9d766b2 Guenter Roeck 2016-07-01 377 case hwmon_pwm_enable:
54187ff9d766b2 Guenter Roeck 2016-07-01 378 if (!(fan_config & MAX31790_FAN_CFG_TACH_INPUT))
dc8dbb4d7672b7 Guenter Roeck 2018-12-10 379 return 0644;
54187ff9d766b2 Guenter Roeck 2016-07-01 380 return 0;
54187ff9d766b2 Guenter Roeck 2016-07-01 381 default:
54187ff9d766b2 Guenter Roeck 2016-07-01 382 return 0;
54187ff9d766b2 Guenter Roeck 2016-07-01 383 }
54187ff9d766b2 Guenter Roeck 2016-07-01 384 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Dan, thanks, I will fix this in v3 of this patch series. Vaclav st 17. 3. 2021 v 6:14 odesílatel Dan Carpenter <dan.carpenter@oracle.com> napsal: > > Hi "Václav, > > url: https://github.com/0day-ci/linux/commits/V-clav-Kubern-t/hwmon-max31790-Rework-to-use-regmap/20210317-015931 > base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next > config: x86_64-randconfig-m001-20210316 (attached as .config) > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > smatch warnings: > drivers/hwmon/max31790.c:263 max31790_fan_is_visible() warn: impossible condition '(fan_config < 0) => (0-255 < 0)' > drivers/hwmon/max31790.c:337 max31790_write_pwm() warn: impossible condition '(fan_config < 0) => (0-255 < 0)' > drivers/hwmon/max31790.c:372 max31790_pwm_is_visible() warn: impossible condition '(fan_config < 0) => (0-255 < 0)' > > vim +263 drivers/hwmon/max31790.c > > 54187ff9d766b2 Guenter Roeck 2016-07-01 257 static umode_t max31790_fan_is_visible(const void *_data, u32 attr, int channel) > 195a4b4298a795 Il Han 2015-08-30 258 { > 54187ff9d766b2 Guenter Roeck 2016-07-01 259 const struct max31790_data *data = _data; > 2c8602cfaeab63 Václav Kubernát 2021-03-16 260 struct regmap *regmap = data->regmap; > 2c8602cfaeab63 Václav Kubernát 2021-03-16 261 u8 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); > 2c8602cfaeab63 Václav Kubernát 2021-03-16 262 > 2c8602cfaeab63 Václav Kubernát 2021-03-16 @263 if (fan_config < 0) > ^^^^^^^^^^^^^^ > A u8 can't be negative. > > 2c8602cfaeab63 Václav Kubernát 2021-03-16 264 return 0; > 54187ff9d766b2 Guenter Roeck 2016-07-01 265 > 54187ff9d766b2 Guenter Roeck 2016-07-01 266 switch (attr) { > 54187ff9d766b2 Guenter Roeck 2016-07-01 267 case hwmon_fan_input: > 54187ff9d766b2 Guenter Roeck 2016-07-01 268 case hwmon_fan_fault: > 54187ff9d766b2 Guenter Roeck 2016-07-01 269 if (channel < NR_CHANNEL || > 54187ff9d766b2 Guenter Roeck 2016-07-01 270 (fan_config & MAX31790_FAN_CFG_TACH_INPUT)) > dc8dbb4d7672b7 Guenter Roeck 2018-12-10 271 return 0444; > 54187ff9d766b2 Guenter Roeck 2016-07-01 272 return 0; > 54187ff9d766b2 Guenter Roeck 2016-07-01 273 case hwmon_fan_target: > 54187ff9d766b2 Guenter Roeck 2016-07-01 274 if (channel < NR_CHANNEL && > 54187ff9d766b2 Guenter Roeck 2016-07-01 275 !(fan_config & MAX31790_FAN_CFG_TACH_INPUT)) > dc8dbb4d7672b7 Guenter Roeck 2018-12-10 276 return 0644; > 54187ff9d766b2 Guenter Roeck 2016-07-01 277 return 0; > 54187ff9d766b2 Guenter Roeck 2016-07-01 278 default: > 54187ff9d766b2 Guenter Roeck 2016-07-01 279 return 0; > 195a4b4298a795 Il Han 2015-08-30 280 } > 195a4b4298a795 Il Han 2015-08-30 281 } > 195a4b4298a795 Il Han 2015-08-30 282 > 54187ff9d766b2 Guenter Roeck 2016-07-01 283 static int max31790_read_pwm(struct device *dev, u32 attr, int channel, > 54187ff9d766b2 Guenter Roeck 2016-07-01 284 long *val) > 195a4b4298a795 Il Han 2015-08-30 285 { > 2c8602cfaeab63 Václav Kubernát 2021-03-16 286 struct max31790_data *data = dev_get_drvdata(dev); > 2c8602cfaeab63 Václav Kubernát 2021-03-16 287 struct regmap *regmap = data->regmap; > 2c8602cfaeab63 Václav Kubernát 2021-03-16 288 int read; > 195a4b4298a795 Il Han 2015-08-30 289 > 195a4b4298a795 Il Han 2015-08-30 290 if (IS_ERR(data)) > 195a4b4298a795 Il Han 2015-08-30 291 return PTR_ERR(data); > 195a4b4298a795 Il Han 2015-08-30 292 > 54187ff9d766b2 Guenter Roeck 2016-07-01 293 switch (attr) { > 54187ff9d766b2 Guenter Roeck 2016-07-01 294 case hwmon_pwm_input: > 2c8602cfaeab63 Václav Kubernát 2021-03-16 295 read = read_reg_word(regmap, MAX31790_REG_PWMOUT(channel)); > 2c8602cfaeab63 Václav Kubernát 2021-03-16 296 if (read < 0) > 2c8602cfaeab63 Václav Kubernát 2021-03-16 297 return read; > 2c8602cfaeab63 Václav Kubernát 2021-03-16 298 > 2c8602cfaeab63 Václav Kubernát 2021-03-16 299 *val = read >> 8; > 54187ff9d766b2 Guenter Roeck 2016-07-01 300 return 0; > 54187ff9d766b2 Guenter Roeck 2016-07-01 301 case hwmon_pwm_enable: > 2c8602cfaeab63 Václav Kubernát 2021-03-16 302 read = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel)); > 2c8602cfaeab63 Václav Kubernát 2021-03-16 303 if (read < 0) > 2c8602cfaeab63 Václav Kubernát 2021-03-16 304 return read; > 2c8602cfaeab63 Václav Kubernát 2021-03-16 305 > 2c8602cfaeab63 Václav Kubernát 2021-03-16 306 if (read & MAX31790_FAN_CFG_RPM_MODE) > 54187ff9d766b2 Guenter Roeck 2016-07-01 307 *val = 2; > 2c8602cfaeab63 Václav Kubernát 2021-03-16 308 else if (read & MAX31790_FAN_CFG_TACH_INPUT_EN) > 54187ff9d766b2 Guenter Roeck 2016-07-01 309 *val = 1; > 195a4b4298a795 Il Han 2015-08-30 310 else > 54187ff9d766b2 Guenter Roeck 2016-07-01 311 *val = 0; > 54187ff9d766b2 Guenter Roeck 2016-07-01 312 return 0; > 54187ff9d766b2 Guenter Roeck 2016-07-01 313 default: > 54187ff9d766b2 Guenter Roeck 2016-07-01 314 return -EOPNOTSUPP; > 54187ff9d766b2 Guenter Roeck 2016-07-01 315 } > 195a4b4298a795 Il Han 2015-08-30 316 } > 195a4b4298a795 Il Han 2015-08-30 317 > 54187ff9d766b2 Guenter Roeck 2016-07-01 318 static int max31790_write_pwm(struct device *dev, u32 attr, int channel, > 54187ff9d766b2 Guenter Roeck 2016-07-01 319 long val) > 195a4b4298a795 Il Han 2015-08-30 320 { > 195a4b4298a795 Il Han 2015-08-30 321 struct max31790_data *data = dev_get_drvdata(dev); > 2c8602cfaeab63 Václav Kubernát 2021-03-16 322 struct regmap *regmap = data->regmap; > 54187ff9d766b2 Guenter Roeck 2016-07-01 323 u8 fan_config; > 54187ff9d766b2 Guenter Roeck 2016-07-01 324 int err = 0; > 195a4b4298a795 Il Han 2015-08-30 325 > 54187ff9d766b2 Guenter Roeck 2016-07-01 326 switch (attr) { > 54187ff9d766b2 Guenter Roeck 2016-07-01 327 case hwmon_pwm_input: > 54187ff9d766b2 Guenter Roeck 2016-07-01 328 if (val < 0 || val > 255) { > 54187ff9d766b2 Guenter Roeck 2016-07-01 329 err = -EINVAL; > 54187ff9d766b2 Guenter Roeck 2016-07-01 330 break; > 54187ff9d766b2 Guenter Roeck 2016-07-01 331 } > 2c8602cfaeab63 Václav Kubernát 2021-03-16 332 err = write_reg_word(regmap, MAX31790_REG_PWMOUT(channel), val << 8); > 195a4b4298a795 Il Han 2015-08-30 333 break; > 54187ff9d766b2 Guenter Roeck 2016-07-01 334 case hwmon_pwm_enable: > 2c8602cfaeab63 Václav Kubernát 2021-03-16 335 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); > 2c8602cfaeab63 Václav Kubernát 2021-03-16 336 > 2c8602cfaeab63 Václav Kubernát 2021-03-16 @337 if (fan_config < 0) > 2c8602cfaeab63 Václav Kubernát 2021-03-16 338 return fan_config; > 2c8602cfaeab63 Václav Kubernát 2021-03-16 339 > 54187ff9d766b2 Guenter Roeck 2016-07-01 340 if (val == 0) { > 54187ff9d766b2 Guenter Roeck 2016-07-01 341 fan_config &= ~(MAX31790_FAN_CFG_TACH_INPUT_EN | > 54187ff9d766b2 Guenter Roeck 2016-07-01 342 MAX31790_FAN_CFG_RPM_MODE); > 54187ff9d766b2 Guenter Roeck 2016-07-01 343 } else if (val == 1) { > 54187ff9d766b2 Guenter Roeck 2016-07-01 344 fan_config = (fan_config | > 54187ff9d766b2 Guenter Roeck 2016-07-01 345 MAX31790_FAN_CFG_TACH_INPUT_EN) & > 54187ff9d766b2 Guenter Roeck 2016-07-01 346 ~MAX31790_FAN_CFG_RPM_MODE; > 54187ff9d766b2 Guenter Roeck 2016-07-01 347 } else if (val == 2) { > 54187ff9d766b2 Guenter Roeck 2016-07-01 348 fan_config |= MAX31790_FAN_CFG_TACH_INPUT_EN | > 54187ff9d766b2 Guenter Roeck 2016-07-01 349 MAX31790_FAN_CFG_RPM_MODE; > 54187ff9d766b2 Guenter Roeck 2016-07-01 350 } else { > 54187ff9d766b2 Guenter Roeck 2016-07-01 351 err = -EINVAL; > 195a4b4298a795 Il Han 2015-08-30 352 break; > 54187ff9d766b2 Guenter Roeck 2016-07-01 353 } > 2c8602cfaeab63 Václav Kubernát 2021-03-16 354 err = regmap_write(regmap, > 54187ff9d766b2 Guenter Roeck 2016-07-01 355 MAX31790_REG_FAN_CONFIG(channel), > 54187ff9d766b2 Guenter Roeck 2016-07-01 356 fan_config); > 195a4b4298a795 Il Han 2015-08-30 357 break; > 195a4b4298a795 Il Han 2015-08-30 358 default: > 54187ff9d766b2 Guenter Roeck 2016-07-01 359 err = -EOPNOTSUPP; > 54187ff9d766b2 Guenter Roeck 2016-07-01 360 break; > 195a4b4298a795 Il Han 2015-08-30 361 } > 195a4b4298a795 Il Han 2015-08-30 362 > 195a4b4298a795 Il Han 2015-08-30 363 return err; > 195a4b4298a795 Il Han 2015-08-30 364 } > 195a4b4298a795 Il Han 2015-08-30 365 > 54187ff9d766b2 Guenter Roeck 2016-07-01 366 static umode_t max31790_pwm_is_visible(const void *_data, u32 attr, int channel) > 195a4b4298a795 Il Han 2015-08-30 367 { > 54187ff9d766b2 Guenter Roeck 2016-07-01 368 const struct max31790_data *data = _data; > 2c8602cfaeab63 Václav Kubernát 2021-03-16 369 struct regmap *regmap = data->regmap; > 2c8602cfaeab63 Václav Kubernát 2021-03-16 370 u8 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); > 2c8602cfaeab63 Václav Kubernát 2021-03-16 371 > 2c8602cfaeab63 Václav Kubernát 2021-03-16 @372 if (fan_config < 0) > 2c8602cfaeab63 Václav Kubernát 2021-03-16 373 return 0; > 54187ff9d766b2 Guenter Roeck 2016-07-01 374 > 54187ff9d766b2 Guenter Roeck 2016-07-01 375 switch (attr) { > 54187ff9d766b2 Guenter Roeck 2016-07-01 376 case hwmon_pwm_input: > 54187ff9d766b2 Guenter Roeck 2016-07-01 377 case hwmon_pwm_enable: > 54187ff9d766b2 Guenter Roeck 2016-07-01 378 if (!(fan_config & MAX31790_FAN_CFG_TACH_INPUT)) > dc8dbb4d7672b7 Guenter Roeck 2018-12-10 379 return 0644; > 54187ff9d766b2 Guenter Roeck 2016-07-01 380 return 0; > 54187ff9d766b2 Guenter Roeck 2016-07-01 381 default: > 54187ff9d766b2 Guenter Roeck 2016-07-01 382 return 0; > 54187ff9d766b2 Guenter Roeck 2016-07-01 383 } > 54187ff9d766b2 Guenter Roeck 2016-07-01 384 } > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Guenter, Thank you for the comments. I will fix the issues in a V3 patch. About the mutex: I was looking at regmap and saw it did locking by itself. But I suppose writing still has to be locked, because the write function writes more than once. I will add the mutex back. Václav út 30. 3. 2021 v 0:27 odesílatel Guenter Roeck <linux@roeck-us.net> napsal: > > On Tue, Mar 16, 2021 at 06:54:58PM +0100, Václav Kubernát wrote: > > Converting the driver to use regmap makes it more generic. It also makes > > it a lot easier to debug through debugfs. > > > > Signed-off-by: Václav Kubernát <kubernat@cesnet.cz> > > Comments are in addition to comments from Dan and 0-day. > > > --- > > drivers/hwmon/Kconfig | 1 + > > drivers/hwmon/max31790.c | 318 ++++++++++++++++++++------------------- > > 2 files changed, 163 insertions(+), 156 deletions(-) > > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > > index 54f04e61fb83..c2ec57672c4e 100644 > > --- a/drivers/hwmon/Kconfig > > +++ b/drivers/hwmon/Kconfig > > @@ -1092,6 +1092,7 @@ config SENSORS_MAX6697 > > config SENSORS_MAX31790 > > tristate "Maxim MAX31790 sensor chip" > > depends on I2C > > + select REGMAP_I2C > > help > > If you say yes here you get support for 6-Channel PWM-Output > > Fan RPM Controller. > > diff --git a/drivers/hwmon/max31790.c b/drivers/hwmon/max31790.c > > index 86e6c71db685..4e5add567890 100644 > > --- a/drivers/hwmon/max31790.c > > +++ b/drivers/hwmon/max31790.c > > @@ -12,6 +12,7 @@ > > #include <linux/init.h> > > #include <linux/jiffies.h> > > #include <linux/module.h> > > +#include <linux/regmap.h> > > #include <linux/slab.h> > > > > /* MAX31790 registers */ > > @@ -46,92 +47,49 @@ > > > > #define NR_CHANNEL 6 > > > > +#define MAX31790_REG_USER_BYTE_67 0x67 > > + > > +#define BULK_TO_U16(msb, lsb) (((msb) << 8) + (lsb)) > > +#define U16_MSB(num) (((num) & 0xFF00) >> 8) > > +#define U16_LSB(num) ((num) & 0x00FF) > > + > > +static const struct regmap_range max31790_ro_range = { > > + .range_min = MAX31790_REG_TACH_COUNT(0), > > + .range_max = MAX31790_REG_PWMOUT(0) - 1, > > +}; > > + > > +static const struct regmap_access_table max31790_wr_table = { > > + .no_ranges = &max31790_ro_range, > > + .n_no_ranges = 1, > > +}; > > + > > +static const struct regmap_range max31790_volatile_ranges[] = { > > + regmap_reg_range(MAX31790_REG_TACH_COUNT(0), MAX31790_REG_TACH_COUNT(12)), > > + regmap_reg_range(MAX31790_REG_FAN_FAULT_STATUS2, MAX31790_REG_FAN_FAULT_STATUS1), > > +}; > > + > > +static const struct regmap_access_table max31790_volatile_table = { > > + .no_ranges = max31790_volatile_ranges, > > + .n_no_ranges = 2, > > + .n_yes_ranges = 0 > > +}; > > + > > +static const struct regmap_config max31790_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > + .reg_stride = 1, > > + .max_register = MAX31790_REG_USER_BYTE_67, > > + .wr_table = &max31790_wr_table, > > + .volatile_table = &max31790_volatile_table > > +}; > > + > > /* > > * Client data (each client gets its own) > > */ > > struct max31790_data { > > - struct i2c_client *client; > > - struct mutex update_lock; > > - bool valid; /* zero until following fields are valid */ > > - unsigned long last_updated; /* in jiffies */ > > - > > - /* register values */ > > - u8 fan_config[NR_CHANNEL]; > > - u8 fan_dynamics[NR_CHANNEL]; > > - u16 fault_status; > > - u16 tach[NR_CHANNEL * 2]; > > - u16 pwm[NR_CHANNEL]; > > - u16 target_count[NR_CHANNEL]; > > + struct regmap *regmap; > > }; > > > > -static struct max31790_data *max31790_update_device(struct device *dev) > > -{ > > - struct max31790_data *data = dev_get_drvdata(dev); > > - struct i2c_client *client = data->client; > > - struct max31790_data *ret = data; > > - int i; > > - int rv; > > - > > - mutex_lock(&data->update_lock); > > - > > - if (time_after(jiffies, data->last_updated + HZ) || !data->valid) { > > - rv = i2c_smbus_read_byte_data(client, > > - MAX31790_REG_FAN_FAULT_STATUS1); > > - if (rv < 0) > > - goto abort; > > - data->fault_status = rv & 0x3F; > > - > > - rv = i2c_smbus_read_byte_data(client, > > - MAX31790_REG_FAN_FAULT_STATUS2); > > - if (rv < 0) > > - goto abort; > > - data->fault_status |= (rv & 0x3F) << 6; > > - > > - for (i = 0; i < NR_CHANNEL; i++) { > > - rv = i2c_smbus_read_word_swapped(client, > > - MAX31790_REG_TACH_COUNT(i)); > > - if (rv < 0) > > - goto abort; > > - data->tach[i] = rv; > > - > > - if (data->fan_config[i] > > - & MAX31790_FAN_CFG_TACH_INPUT) { > > - rv = i2c_smbus_read_word_swapped(client, > > - MAX31790_REG_TACH_COUNT(NR_CHANNEL > > - + i)); > > - if (rv < 0) > > - goto abort; > > - data->tach[NR_CHANNEL + i] = rv; > > - } else { > > - rv = i2c_smbus_read_word_swapped(client, > > - MAX31790_REG_PWMOUT(i)); > > - if (rv < 0) > > - goto abort; > > - data->pwm[i] = rv; > > - > > - rv = i2c_smbus_read_word_swapped(client, > > - MAX31790_REG_TARGET_COUNT(i)); > > - if (rv < 0) > > - goto abort; > > - data->target_count[i] = rv; > > - } > > - } > > - > > - data->last_updated = jiffies; > > - data->valid = true; > > - } > > - goto done; > > - > > -abort: > > - data->valid = false; > > - ret = ERR_PTR(rv); > > - > > -done: > > - mutex_unlock(&data->update_lock); > > - > > - return ret; > > -} > > - > > static const u8 tach_period[8] = { 1, 2, 4, 8, 16, 32, 32, 32 }; > > > > static u8 get_tach_period(u8 fan_dynamics) > > @@ -159,28 +117,89 @@ static u8 bits_for_tach_period(int rpm) > > return bits; > > } > > > > +static int read_reg_byte(struct regmap *regmap, u8 reg) > > +{ > > + int rv; > > + int val; > > + > > + rv = regmap_read(regmap, reg, &val); > > + > > lease no empty line between assignment and check. > > > + if (rv < 0) > > + return rv; > > + > > + return val; > > +} > > + > > +static int read_reg_word(struct regmap *regmap, u8 reg) > > +{ > > + int rv; > > + u8 val_bulk[2]; > > + > > + rv = regmap_bulk_read(regmap, reg, val_bulk, 2); > > + if (rv < 0) > > + return rv; > > + > > + return BULK_TO_U16(val_bulk[0], val_bulk[1]); > > +} > > + > > +static int write_reg_word(struct regmap *regmap, u8 reg, u16 val) > > +{ > > + u8 bulk_val[2]; > > + > > + bulk_val[0] = U16_MSB(val); > > + bulk_val[1] = U16_LSB(val); > > + > > + return regmap_bulk_write(regmap, reg, bulk_val, 2); > > +} > > + > > static int max31790_read_fan(struct device *dev, u32 attr, int channel, > > long *val) > > { > > - struct max31790_data *data = max31790_update_device(dev); > > - int sr, rpm; > > + struct max31790_data *data = dev_get_drvdata(dev); > > + struct regmap *regmap = data->regmap; > > + int rpm, dynamics, tach, fault; > > > > if (IS_ERR(data)) > > return PTR_ERR(data); > > Now unnecessary. > > > > > switch (attr) { > > case hwmon_fan_input: > > - sr = get_tach_period(data->fan_dynamics[channel]); > > - rpm = RPM_FROM_REG(data->tach[channel], sr); > > + dynamics = read_reg_byte(regmap, MAX31790_REG_FAN_DYNAMICS(channel)); > > + if (dynamics < 0) > > + return dynamics; > > + > > + tach = read_reg_word(regmap, MAX31790_REG_TACH_COUNT(channel)); > > + if (tach < 0) > > + return tach; > > + > > + rpm = RPM_FROM_REG(tach, get_tach_period(dynamics)); > > *val = rpm; > > *val = RPM_FROM_REG(tach, get_tach_period(dynamics)); > > > return 0; > > case hwmon_fan_target: > > - sr = get_tach_period(data->fan_dynamics[channel]); > > - rpm = RPM_FROM_REG(data->target_count[channel], sr); > > + dynamics = read_reg_byte(regmap, MAX31790_REG_FAN_DYNAMICS(channel)); > > + if (dynamics < 0) > > + return dynamics; > > + > > + tach = read_reg_word(regmap, MAX31790_REG_TARGET_COUNT(channel)); > > + if (tach < 0) > > + return tach; > > + > > + rpm = RPM_FROM_REG(tach, get_tach_period(dynamics)); > > *val = rpm; > > *val = RPM_FROM_REG(tach, get_tach_period(dynamics)); > > > return 0; > > case hwmon_fan_fault: > > - *val = !!(data->fault_status & (1 << channel)); > > + if (channel > 6) > > + fault = read_reg_byte(regmap, MAX31790_REG_FAN_FAULT_STATUS2); > > + else > > + fault = read_reg_byte(regmap, MAX31790_REG_FAN_FAULT_STATUS1); > > + > > + if (fault < 0) > > + return fault; > > + > > + if (channel > 6) > > + *val = !!(fault & (1 << (channel - 6))); > > + else > > + *val = !!(fault & (1 << channel)); > > return 0; > > default: > > return -EOPNOTSUPP; > > @@ -191,52 +210,58 @@ static int max31790_write_fan(struct device *dev, u32 attr, int channel, > > long val) > > { > > struct max31790_data *data = dev_get_drvdata(dev); > > - struct i2c_client *client = data->client; > > + struct regmap *regmap = data->regmap; > > int target_count; > > int err = 0; > > u8 bits; > > int sr; > > - > > - mutex_lock(&data->update_lock); > > + int fan_dynamics; > > > > switch (attr) { > > case hwmon_fan_target: > > val = clamp_val(val, FAN_RPM_MIN, FAN_RPM_MAX); > > bits = bits_for_tach_period(val); > > - data->fan_dynamics[channel] = > > - ((data->fan_dynamics[channel] & > > + fan_dynamics = read_reg_byte(regmap, MAX31790_REG_FAN_DYNAMICS(channel)); > > + > Unnecessary empty line. > > > + if (fan_dynamics < 0) > > + return fan_dynamics; > > + > > + fan_dynamics = > > + ((fan_dynamics & > > ~MAX31790_FAN_DYN_SR_MASK) | > > (bits << MAX31790_FAN_DYN_SR_SHIFT)); > > - err = i2c_smbus_write_byte_data(client, > > - MAX31790_REG_FAN_DYNAMICS(channel), > > - data->fan_dynamics[channel]); > > + err = regmap_write(regmap, > > + MAX31790_REG_FAN_DYNAMICS(channel), > > + fan_dynamics); > > if (err < 0) > > break; > > > > - sr = get_tach_period(data->fan_dynamics[channel]); > > + sr = get_tach_period(fan_dynamics); > > target_count = RPM_TO_REG(val, sr); > > target_count = clamp_val(target_count, 0x1, 0x7FF); > > > > - data->target_count[channel] = target_count << 5; > > + target_count = target_count << 5; > > > > - err = i2c_smbus_write_word_swapped(client, > > - MAX31790_REG_TARGET_COUNT(channel), > > - data->target_count[channel]); > > + err = write_reg_word(regmap, > > + MAX31790_REG_TARGET_COUNT(channel), > > + target_count); > > break; > > default: > > err = -EOPNOTSUPP; > > break; > > } > > > > - mutex_unlock(&data->update_lock); > > - > Why is this lock no longer required ? There are still multiple writes > when writing hwmon_fan_target. > > > return err; > > } > > > > static umode_t max31790_fan_is_visible(const void *_data, u32 attr, int channel) > > { > > const struct max31790_data *data = _data; > > - u8 fan_config = data->fan_config[channel % NR_CHANNEL]; > > + struct regmap *regmap = data->regmap; > > + u8 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); > > + > > + if (fan_config < 0) > > + return 0; > > fan_config needs to be int. Also, this is a poor way of handling > this problem. Since fan_config does not change dynamically, > this is one set of values that would make sense to keep cached > locally. > > > > > switch (attr) { > > case hwmon_fan_input: > > @@ -258,22 +283,29 @@ static umode_t max31790_fan_is_visible(const void *_data, u32 attr, int channel) > > static int max31790_read_pwm(struct device *dev, u32 attr, int channel, > > long *val) > > { > > - struct max31790_data *data = max31790_update_device(dev); > > - u8 fan_config; > > + struct max31790_data *data = dev_get_drvdata(dev); > > + struct regmap *regmap = data->regmap; > > + int read; > > > > if (IS_ERR(data)) > > return PTR_ERR(data); > > Now unnecessary. > > > > > - fan_config = data->fan_config[channel]; > > - > > switch (attr) { > > case hwmon_pwm_input: > > - *val = data->pwm[channel] >> 8; > > + read = read_reg_word(regmap, MAX31790_REG_PWMOUT(channel)); > > + if (read < 0) > > + return read; > > + > > + *val = read >> 8; > > return 0; > > case hwmon_pwm_enable: > > - if (fan_config & MAX31790_FAN_CFG_RPM_MODE) > > + read = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel)); > > + if (read < 0) > > + return read; > > + > > + if (read & MAX31790_FAN_CFG_RPM_MODE) > > *val = 2; > > - else if (fan_config & MAX31790_FAN_CFG_TACH_INPUT_EN) > > + else if (read & MAX31790_FAN_CFG_TACH_INPUT_EN) > > *val = 1; > > else > > *val = 0; > > @@ -287,25 +319,24 @@ static int max31790_write_pwm(struct device *dev, u32 attr, int channel, > > long val) > > { > > struct max31790_data *data = dev_get_drvdata(dev); > > - struct i2c_client *client = data->client; > > + struct regmap *regmap = data->regmap; > > u8 fan_config; > > int err = 0; > > > > - mutex_lock(&data->update_lock); > > - > > switch (attr) { > > case hwmon_pwm_input: > > if (val < 0 || val > 255) { > > err = -EINVAL; > > break; > > } > > - data->pwm[channel] = val << 8; > > - err = i2c_smbus_write_word_swapped(client, > > - MAX31790_REG_PWMOUT(channel), > > - data->pwm[channel]); > > + err = write_reg_word(regmap, MAX31790_REG_PWMOUT(channel), val << 8); > > break; > > case hwmon_pwm_enable: > > - fan_config = data->fan_config[channel]; > > + fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); > > + > > + if (fan_config < 0) > > + return fan_config; > > + > > if (val == 0) { > > fan_config &= ~(MAX31790_FAN_CFG_TACH_INPUT_EN | > > MAX31790_FAN_CFG_RPM_MODE); > > @@ -320,25 +351,26 @@ static int max31790_write_pwm(struct device *dev, u32 attr, int channel, > > err = -EINVAL; > > break; > > } > > - data->fan_config[channel] = fan_config; > > - err = i2c_smbus_write_byte_data(client, > > - MAX31790_REG_FAN_CONFIG(channel), > > - fan_config); > > + err = regmap_write(regmap, > > + MAX31790_REG_FAN_CONFIG(channel), > > + fan_config); > > break; > > default: > > err = -EOPNOTSUPP; > > break; > > } > > > > - mutex_unlock(&data->update_lock); > > - > Are you sure this mutex is no longer needed here, ie that there > can not be an interaction with multiple writes from multiple processes > at the same time ? > > > return err; > > } > > > > static umode_t max31790_pwm_is_visible(const void *_data, u32 attr, int channel) > > { > > const struct max31790_data *data = _data; > > - u8 fan_config = data->fan_config[channel]; > > + struct regmap *regmap = data->regmap; > > + u8 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); > > + > > + if (fan_config < 0) > > + return 0; > > int problem again. > > > > > switch (attr) { > > case hwmon_pwm_input: > > @@ -426,35 +458,12 @@ static const struct hwmon_chip_info max31790_chip_info = { > > .info = max31790_info, > > }; > > > > -static int max31790_init_client(struct i2c_client *client, > > - struct max31790_data *data) > > -{ > > - int i, rv; > > - > > - for (i = 0; i < NR_CHANNEL; i++) { > > - rv = i2c_smbus_read_byte_data(client, > > - MAX31790_REG_FAN_CONFIG(i)); > > - if (rv < 0) > > - return rv; > > - data->fan_config[i] = rv; > > - > > - rv = i2c_smbus_read_byte_data(client, > > - MAX31790_REG_FAN_DYNAMICS(i)); > > - if (rv < 0) > > - return rv; > > - data->fan_dynamics[i] = rv; > > The above "cached" values are static, and it did make sense to keep those > locally to avoid requiring unnecessary error handling (and to detect issues > with the chip early). > > > - } > > - > > - return 0; > > -} > > - > > static int max31790_probe(struct i2c_client *client) > > { > > struct i2c_adapter *adapter = client->adapter; > > struct device *dev = &client->dev; > > struct max31790_data *data; > > struct device *hwmon_dev; > > - int err; > > > > if (!i2c_check_functionality(adapter, > > I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA)) > > @@ -464,15 +473,12 @@ static int max31790_probe(struct i2c_client *client) > > if (!data) > > return -ENOMEM; > > > > - data->client = client; > > - mutex_init(&data->update_lock); > > + data->regmap = devm_regmap_init_i2c(client, &max31790_regmap_config); > > > > - /* > > - * Initialize the max31790 chip > > - */ > > - err = max31790_init_client(client, data); > > - if (err) > > - return err; > > + if (IS_ERR(data->regmap)) { > > + dev_err(dev, "failed to allocate register map\n"); > > + return PTR_ERR(data->regmap); > > + } > > > > hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, > > data, út 30. 3. 2021 v 0:27 odesílatel Guenter Roeck <linux@roeck-us.net> napsal: > > On Tue, Mar 16, 2021 at 06:54:58PM +0100, Václav Kubernát wrote: > > Converting the driver to use regmap makes it more generic. It also makes > > it a lot easier to debug through debugfs. > > > > Signed-off-by: Václav Kubernát <kubernat@cesnet.cz> > > Comments are in addition to comments from Dan and 0-day. > > > --- > > drivers/hwmon/Kconfig | 1 + > > drivers/hwmon/max31790.c | 318 ++++++++++++++++++++------------------- > > 2 files changed, 163 insertions(+), 156 deletions(-) > > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > > index 54f04e61fb83..c2ec57672c4e 100644 > > --- a/drivers/hwmon/Kconfig > > +++ b/drivers/hwmon/Kconfig > > @@ -1092,6 +1092,7 @@ config SENSORS_MAX6697 > > config SENSORS_MAX31790 > > tristate "Maxim MAX31790 sensor chip" > > depends on I2C > > + select REGMAP_I2C > > help > > If you say yes here you get support for 6-Channel PWM-Output > > Fan RPM Controller. > > diff --git a/drivers/hwmon/max31790.c b/drivers/hwmon/max31790.c > > index 86e6c71db685..4e5add567890 100644 > > --- a/drivers/hwmon/max31790.c > > +++ b/drivers/hwmon/max31790.c > > @@ -12,6 +12,7 @@ > > #include <linux/init.h> > > #include <linux/jiffies.h> > > #include <linux/module.h> > > +#include <linux/regmap.h> > > #include <linux/slab.h> > > > > /* MAX31790 registers */ > > @@ -46,92 +47,49 @@ > > > > #define NR_CHANNEL 6 > > > > +#define MAX31790_REG_USER_BYTE_67 0x67 > > + > > +#define BULK_TO_U16(msb, lsb) (((msb) << 8) + (lsb)) > > +#define U16_MSB(num) (((num) & 0xFF00) >> 8) > > +#define U16_LSB(num) ((num) & 0x00FF) > > + > > +static const struct regmap_range max31790_ro_range = { > > + .range_min = MAX31790_REG_TACH_COUNT(0), > > + .range_max = MAX31790_REG_PWMOUT(0) - 1, > > +}; > > + > > +static const struct regmap_access_table max31790_wr_table = { > > + .no_ranges = &max31790_ro_range, > > + .n_no_ranges = 1, > > +}; > > + > > +static const struct regmap_range max31790_volatile_ranges[] = { > > + regmap_reg_range(MAX31790_REG_TACH_COUNT(0), MAX31790_REG_TACH_COUNT(12)), > > + regmap_reg_range(MAX31790_REG_FAN_FAULT_STATUS2, MAX31790_REG_FAN_FAULT_STATUS1), > > +}; > > + > > +static const struct regmap_access_table max31790_volatile_table = { > > + .no_ranges = max31790_volatile_ranges, > > + .n_no_ranges = 2, > > + .n_yes_ranges = 0 > > +}; > > + > > +static const struct regmap_config max31790_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > + .reg_stride = 1, > > + .max_register = MAX31790_REG_USER_BYTE_67, > > + .wr_table = &max31790_wr_table, > > + .volatile_table = &max31790_volatile_table > > +}; > > + > > /* > > * Client data (each client gets its own) > > */ > > struct max31790_data { > > - struct i2c_client *client; > > - struct mutex update_lock; > > - bool valid; /* zero until following fields are valid */ > > - unsigned long last_updated; /* in jiffies */ > > - > > - /* register values */ > > - u8 fan_config[NR_CHANNEL]; > > - u8 fan_dynamics[NR_CHANNEL]; > > - u16 fault_status; > > - u16 tach[NR_CHANNEL * 2]; > > - u16 pwm[NR_CHANNEL]; > > - u16 target_count[NR_CHANNEL]; > > + struct regmap *regmap; > > }; > > > > -static struct max31790_data *max31790_update_device(struct device *dev) > > -{ > > - struct max31790_data *data = dev_get_drvdata(dev); > > - struct i2c_client *client = data->client; > > - struct max31790_data *ret = data; > > - int i; > > - int rv; > > - > > - mutex_lock(&data->update_lock); > > - > > - if (time_after(jiffies, data->last_updated + HZ) || !data->valid) { > > - rv = i2c_smbus_read_byte_data(client, > > - MAX31790_REG_FAN_FAULT_STATUS1); > > - if (rv < 0) > > - goto abort; > > - data->fault_status = rv & 0x3F; > > - > > - rv = i2c_smbus_read_byte_data(client, > > - MAX31790_REG_FAN_FAULT_STATUS2); > > - if (rv < 0) > > - goto abort; > > - data->fault_status |= (rv & 0x3F) << 6; > > - > > - for (i = 0; i < NR_CHANNEL; i++) { > > - rv = i2c_smbus_read_word_swapped(client, > > - MAX31790_REG_TACH_COUNT(i)); > > - if (rv < 0) > > - goto abort; > > - data->tach[i] = rv; > > - > > - if (data->fan_config[i] > > - & MAX31790_FAN_CFG_TACH_INPUT) { > > - rv = i2c_smbus_read_word_swapped(client, > > - MAX31790_REG_TACH_COUNT(NR_CHANNEL > > - + i)); > > - if (rv < 0) > > - goto abort; > > - data->tach[NR_CHANNEL + i] = rv; > > - } else { > > - rv = i2c_smbus_read_word_swapped(client, > > - MAX31790_REG_PWMOUT(i)); > > - if (rv < 0) > > - goto abort; > > - data->pwm[i] = rv; > > - > > - rv = i2c_smbus_read_word_swapped(client, > > - MAX31790_REG_TARGET_COUNT(i)); > > - if (rv < 0) > > - goto abort; > > - data->target_count[i] = rv; > > - } > > - } > > - > > - data->last_updated = jiffies; > > - data->valid = true; > > - } > > - goto done; > > - > > -abort: > > - data->valid = false; > > - ret = ERR_PTR(rv); > > - > > -done: > > - mutex_unlock(&data->update_lock); > > - > > - return ret; > > -} > > - > > static const u8 tach_period[8] = { 1, 2, 4, 8, 16, 32, 32, 32 }; > > > > static u8 get_tach_period(u8 fan_dynamics) > > @@ -159,28 +117,89 @@ static u8 bits_for_tach_period(int rpm) > > return bits; > > } > > > > +static int read_reg_byte(struct regmap *regmap, u8 reg) > > +{ > > + int rv; > > + int val; > > + > > + rv = regmap_read(regmap, reg, &val); > > + > > lease no empty line between assignment and check. > > > + if (rv < 0) > > + return rv; > > + > > + return val; > > +} > > + > > +static int read_reg_word(struct regmap *regmap, u8 reg) > > +{ > > + int rv; > > + u8 val_bulk[2]; > > + > > + rv = regmap_bulk_read(regmap, reg, val_bulk, 2); > > + if (rv < 0) > > + return rv; > > + > > + return BULK_TO_U16(val_bulk[0], val_bulk[1]); > > +} > > + > > +static int write_reg_word(struct regmap *regmap, u8 reg, u16 val) > > +{ > > + u8 bulk_val[2]; > > + > > + bulk_val[0] = U16_MSB(val); > > + bulk_val[1] = U16_LSB(val); > > + > > + return regmap_bulk_write(regmap, reg, bulk_val, 2); > > +} > > + > > static int max31790_read_fan(struct device *dev, u32 attr, int channel, > > long *val) > > { > > - struct max31790_data *data = max31790_update_device(dev); > > - int sr, rpm; > > + struct max31790_data *data = dev_get_drvdata(dev); > > + struct regmap *regmap = data->regmap; > > + int rpm, dynamics, tach, fault; > > > > if (IS_ERR(data)) > > return PTR_ERR(data); > > Now unnecessary. > > > > > switch (attr) { > > case hwmon_fan_input: > > - sr = get_tach_period(data->fan_dynamics[channel]); > > - rpm = RPM_FROM_REG(data->tach[channel], sr); > > + dynamics = read_reg_byte(regmap, MAX31790_REG_FAN_DYNAMICS(channel)); > > + if (dynamics < 0) > > + return dynamics; > > + > > + tach = read_reg_word(regmap, MAX31790_REG_TACH_COUNT(channel)); > > + if (tach < 0) > > + return tach; > > + > > + rpm = RPM_FROM_REG(tach, get_tach_period(dynamics)); > > *val = rpm; > > *val = RPM_FROM_REG(tach, get_tach_period(dynamics)); > > > return 0; > > case hwmon_fan_target: > > - sr = get_tach_period(data->fan_dynamics[channel]); > > - rpm = RPM_FROM_REG(data->target_count[channel], sr); > > + dynamics = read_reg_byte(regmap, MAX31790_REG_FAN_DYNAMICS(channel)); > > + if (dynamics < 0) > > + return dynamics; > > + > > + tach = read_reg_word(regmap, MAX31790_REG_TARGET_COUNT(channel)); > > + if (tach < 0) > > + return tach; > > + > > + rpm = RPM_FROM_REG(tach, get_tach_period(dynamics)); > > *val = rpm; > > *val = RPM_FROM_REG(tach, get_tach_period(dynamics)); > > > return 0; > > case hwmon_fan_fault: > > - *val = !!(data->fault_status & (1 << channel)); > > + if (channel > 6) > > + fault = read_reg_byte(regmap, MAX31790_REG_FAN_FAULT_STATUS2); > > + else > > + fault = read_reg_byte(regmap, MAX31790_REG_FAN_FAULT_STATUS1); > > + > > + if (fault < 0) > > + return fault; > > + > > + if (channel > 6) > > + *val = !!(fault & (1 << (channel - 6))); > > + else > > + *val = !!(fault & (1 << channel)); > > return 0; > > default: > > return -EOPNOTSUPP; > > @@ -191,52 +210,58 @@ static int max31790_write_fan(struct device *dev, u32 attr, int channel, > > long val) > > { > > struct max31790_data *data = dev_get_drvdata(dev); > > - struct i2c_client *client = data->client; > > + struct regmap *regmap = data->regmap; > > int target_count; > > int err = 0; > > u8 bits; > > int sr; > > - > > - mutex_lock(&data->update_lock); > > + int fan_dynamics; > > > > switch (attr) { > > case hwmon_fan_target: > > val = clamp_val(val, FAN_RPM_MIN, FAN_RPM_MAX); > > bits = bits_for_tach_period(val); > > - data->fan_dynamics[channel] = > > - ((data->fan_dynamics[channel] & > > + fan_dynamics = read_reg_byte(regmap, MAX31790_REG_FAN_DYNAMICS(channel)); > > + > Unnecessary empty line. > > > + if (fan_dynamics < 0) > > + return fan_dynamics; > > + > > + fan_dynamics = > > + ((fan_dynamics & > > ~MAX31790_FAN_DYN_SR_MASK) | > > (bits << MAX31790_FAN_DYN_SR_SHIFT)); > > - err = i2c_smbus_write_byte_data(client, > > - MAX31790_REG_FAN_DYNAMICS(channel), > > - data->fan_dynamics[channel]); > > + err = regmap_write(regmap, > > + MAX31790_REG_FAN_DYNAMICS(channel), > > + fan_dynamics); > > if (err < 0) > > break; > > > > - sr = get_tach_period(data->fan_dynamics[channel]); > > + sr = get_tach_period(fan_dynamics); > > target_count = RPM_TO_REG(val, sr); > > target_count = clamp_val(target_count, 0x1, 0x7FF); > > > > - data->target_count[channel] = target_count << 5; > > + target_count = target_count << 5; > > > > - err = i2c_smbus_write_word_swapped(client, > > - MAX31790_REG_TARGET_COUNT(channel), > > - data->target_count[channel]); > > + err = write_reg_word(regmap, > > + MAX31790_REG_TARGET_COUNT(channel), > > + target_count); > > break; > > default: > > err = -EOPNOTSUPP; > > break; > > } > > > > - mutex_unlock(&data->update_lock); > > - > Why is this lock no longer required ? There are still multiple writes > when writing hwmon_fan_target. > > > return err; > > } > > > > static umode_t max31790_fan_is_visible(const void *_data, u32 attr, int channel) > > { > > const struct max31790_data *data = _data; > > - u8 fan_config = data->fan_config[channel % NR_CHANNEL]; > > + struct regmap *regmap = data->regmap; > > + u8 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); > > + > > + if (fan_config < 0) > > + return 0; > > fan_config needs to be int. Also, this is a poor way of handling > this problem. Since fan_config does not change dynamically, > this is one set of values that would make sense to keep cached > locally. > > > > > switch (attr) { > > case hwmon_fan_input: > > @@ -258,22 +283,29 @@ static umode_t max31790_fan_is_visible(const void *_data, u32 attr, int channel) > > static int max31790_read_pwm(struct device *dev, u32 attr, int channel, > > long *val) > > { > > - struct max31790_data *data = max31790_update_device(dev); > > - u8 fan_config; > > + struct max31790_data *data = dev_get_drvdata(dev); > > + struct regmap *regmap = data->regmap; > > + int read; > > > > if (IS_ERR(data)) > > return PTR_ERR(data); > > Now unnecessary. > > > > > - fan_config = data->fan_config[channel]; > > - > > switch (attr) { > > case hwmon_pwm_input: > > - *val = data->pwm[channel] >> 8; > > + read = read_reg_word(regmap, MAX31790_REG_PWMOUT(channel)); > > + if (read < 0) > > + return read; > > + > > + *val = read >> 8; > > return 0; > > case hwmon_pwm_enable: > > - if (fan_config & MAX31790_FAN_CFG_RPM_MODE) > > + read = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel)); > > + if (read < 0) > > + return read; > > + > > + if (read & MAX31790_FAN_CFG_RPM_MODE) > > *val = 2; > > - else if (fan_config & MAX31790_FAN_CFG_TACH_INPUT_EN) > > + else if (read & MAX31790_FAN_CFG_TACH_INPUT_EN) > > *val = 1; > > else > > *val = 0; > > @@ -287,25 +319,24 @@ static int max31790_write_pwm(struct device *dev, u32 attr, int channel, > > long val) > > { > > struct max31790_data *data = dev_get_drvdata(dev); > > - struct i2c_client *client = data->client; > > + struct regmap *regmap = data->regmap; > > u8 fan_config; > > int err = 0; > > > > - mutex_lock(&data->update_lock); > > - > > switch (attr) { > > case hwmon_pwm_input: > > if (val < 0 || val > 255) { > > err = -EINVAL; > > break; > > } > > - data->pwm[channel] = val << 8; > > - err = i2c_smbus_write_word_swapped(client, > > - MAX31790_REG_PWMOUT(channel), > > - data->pwm[channel]); > > + err = write_reg_word(regmap, MAX31790_REG_PWMOUT(channel), val << 8); > > break; > > case hwmon_pwm_enable: > > - fan_config = data->fan_config[channel]; > > + fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); > > + > > + if (fan_config < 0) > > + return fan_config; > > + > > if (val == 0) { > > fan_config &= ~(MAX31790_FAN_CFG_TACH_INPUT_EN | > > MAX31790_FAN_CFG_RPM_MODE); > > @@ -320,25 +351,26 @@ static int max31790_write_pwm(struct device *dev, u32 attr, int channel, > > err = -EINVAL; > > break; > > } > > - data->fan_config[channel] = fan_config; > > - err = i2c_smbus_write_byte_data(client, > > - MAX31790_REG_FAN_CONFIG(channel), > > - fan_config); > > + err = regmap_write(regmap, > > + MAX31790_REG_FAN_CONFIG(channel), > > + fan_config); > > break; > > default: > > err = -EOPNOTSUPP; > > break; > > } > > > > - mutex_unlock(&data->update_lock); > > - > Are you sure this mutex is no longer needed here, ie that there > can not be an interaction with multiple writes from multiple processes > at the same time ? > > > return err; > > } > > > > static umode_t max31790_pwm_is_visible(const void *_data, u32 attr, int channel) > > { > > const struct max31790_data *data = _data; > > - u8 fan_config = data->fan_config[channel]; > > + struct regmap *regmap = data->regmap; > > + u8 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); > > + > > + if (fan_config < 0) > > + return 0; > > int problem again. > > > > > switch (attr) { > > case hwmon_pwm_input: > > @@ -426,35 +458,12 @@ static const struct hwmon_chip_info max31790_chip_info = { > > .info = max31790_info, > > }; > > > > -static int max31790_init_client(struct i2c_client *client, > > - struct max31790_data *data) > > -{ > > - int i, rv; > > - > > - for (i = 0; i < NR_CHANNEL; i++) { > > - rv = i2c_smbus_read_byte_data(client, > > - MAX31790_REG_FAN_CONFIG(i)); > > - if (rv < 0) > > - return rv; > > - data->fan_config[i] = rv; > > - > > - rv = i2c_smbus_read_byte_data(client, > > - MAX31790_REG_FAN_DYNAMICS(i)); > > - if (rv < 0) > > - return rv; > > - data->fan_dynamics[i] = rv; > > The above "cached" values are static, and it did make sense to keep those > locally to avoid requiring unnecessary error handling (and to detect issues > with the chip early). > > > - } > > - > > - return 0; > > -} > > - > > static int max31790_probe(struct i2c_client *client) > > { > > struct i2c_adapter *adapter = client->adapter; > > struct device *dev = &client->dev; > > struct max31790_data *data; > > struct device *hwmon_dev; > > - int err; > > > > if (!i2c_check_functionality(adapter, > > I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA)) > > @@ -464,15 +473,12 @@ static int max31790_probe(struct i2c_client *client) > > if (!data) > > return -ENOMEM; > > > > - data->client = client; > > - mutex_init(&data->update_lock); > > + data->regmap = devm_regmap_init_i2c(client, &max31790_regmap_config); > > > > - /* > > - * Initialize the max31790 chip > > - */ > > - err = max31790_init_client(client, data); > > - if (err) > > - return err; > > + if (IS_ERR(data->regmap)) { > > + dev_err(dev, "failed to allocate register map\n"); > > + return PTR_ERR(data->regmap); > > + } > > > > hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, > > data,
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 54f04e61fb83..c2ec57672c4e 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1092,6 +1092,7 @@ config SENSORS_MAX6697 config SENSORS_MAX31790 tristate "Maxim MAX31790 sensor chip" depends on I2C + select REGMAP_I2C help If you say yes here you get support for 6-Channel PWM-Output Fan RPM Controller. diff --git a/drivers/hwmon/max31790.c b/drivers/hwmon/max31790.c index 86e6c71db685..4e5add567890 100644 --- a/drivers/hwmon/max31790.c +++ b/drivers/hwmon/max31790.c @@ -12,6 +12,7 @@ #include <linux/init.h> #include <linux/jiffies.h> #include <linux/module.h> +#include <linux/regmap.h> #include <linux/slab.h> /* MAX31790 registers */ @@ -46,92 +47,49 @@ #define NR_CHANNEL 6 +#define MAX31790_REG_USER_BYTE_67 0x67 + +#define BULK_TO_U16(msb, lsb) (((msb) << 8) + (lsb)) +#define U16_MSB(num) (((num) & 0xFF00) >> 8) +#define U16_LSB(num) ((num) & 0x00FF) + +static const struct regmap_range max31790_ro_range = { + .range_min = MAX31790_REG_TACH_COUNT(0), + .range_max = MAX31790_REG_PWMOUT(0) - 1, +}; + +static const struct regmap_access_table max31790_wr_table = { + .no_ranges = &max31790_ro_range, + .n_no_ranges = 1, +}; + +static const struct regmap_range max31790_volatile_ranges[] = { + regmap_reg_range(MAX31790_REG_TACH_COUNT(0), MAX31790_REG_TACH_COUNT(12)), + regmap_reg_range(MAX31790_REG_FAN_FAULT_STATUS2, MAX31790_REG_FAN_FAULT_STATUS1), +}; + +static const struct regmap_access_table max31790_volatile_table = { + .no_ranges = max31790_volatile_ranges, + .n_no_ranges = 2, + .n_yes_ranges = 0 +}; + +static const struct regmap_config max31790_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .reg_stride = 1, + .max_register = MAX31790_REG_USER_BYTE_67, + .wr_table = &max31790_wr_table, + .volatile_table = &max31790_volatile_table +}; + /* * Client data (each client gets its own) */ struct max31790_data { - struct i2c_client *client; - struct mutex update_lock; - bool valid; /* zero until following fields are valid */ - unsigned long last_updated; /* in jiffies */ - - /* register values */ - u8 fan_config[NR_CHANNEL]; - u8 fan_dynamics[NR_CHANNEL]; - u16 fault_status; - u16 tach[NR_CHANNEL * 2]; - u16 pwm[NR_CHANNEL]; - u16 target_count[NR_CHANNEL]; + struct regmap *regmap; }; -static struct max31790_data *max31790_update_device(struct device *dev) -{ - struct max31790_data *data = dev_get_drvdata(dev); - struct i2c_client *client = data->client; - struct max31790_data *ret = data; - int i; - int rv; - - mutex_lock(&data->update_lock); - - if (time_after(jiffies, data->last_updated + HZ) || !data->valid) { - rv = i2c_smbus_read_byte_data(client, - MAX31790_REG_FAN_FAULT_STATUS1); - if (rv < 0) - goto abort; - data->fault_status = rv & 0x3F; - - rv = i2c_smbus_read_byte_data(client, - MAX31790_REG_FAN_FAULT_STATUS2); - if (rv < 0) - goto abort; - data->fault_status |= (rv & 0x3F) << 6; - - for (i = 0; i < NR_CHANNEL; i++) { - rv = i2c_smbus_read_word_swapped(client, - MAX31790_REG_TACH_COUNT(i)); - if (rv < 0) - goto abort; - data->tach[i] = rv; - - if (data->fan_config[i] - & MAX31790_FAN_CFG_TACH_INPUT) { - rv = i2c_smbus_read_word_swapped(client, - MAX31790_REG_TACH_COUNT(NR_CHANNEL - + i)); - if (rv < 0) - goto abort; - data->tach[NR_CHANNEL + i] = rv; - } else { - rv = i2c_smbus_read_word_swapped(client, - MAX31790_REG_PWMOUT(i)); - if (rv < 0) - goto abort; - data->pwm[i] = rv; - - rv = i2c_smbus_read_word_swapped(client, - MAX31790_REG_TARGET_COUNT(i)); - if (rv < 0) - goto abort; - data->target_count[i] = rv; - } - } - - data->last_updated = jiffies; - data->valid = true; - } - goto done; - -abort: - data->valid = false; - ret = ERR_PTR(rv); - -done: - mutex_unlock(&data->update_lock); - - return ret; -} - static const u8 tach_period[8] = { 1, 2, 4, 8, 16, 32, 32, 32 }; static u8 get_tach_period(u8 fan_dynamics) @@ -159,28 +117,89 @@ static u8 bits_for_tach_period(int rpm) return bits; } +static int read_reg_byte(struct regmap *regmap, u8 reg) +{ + int rv; + int val; + + rv = regmap_read(regmap, reg, &val); + + if (rv < 0) + return rv; + + return val; +} + +static int read_reg_word(struct regmap *regmap, u8 reg) +{ + int rv; + u8 val_bulk[2]; + + rv = regmap_bulk_read(regmap, reg, val_bulk, 2); + if (rv < 0) + return rv; + + return BULK_TO_U16(val_bulk[0], val_bulk[1]); +} + +static int write_reg_word(struct regmap *regmap, u8 reg, u16 val) +{ + u8 bulk_val[2]; + + bulk_val[0] = U16_MSB(val); + bulk_val[1] = U16_LSB(val); + + return regmap_bulk_write(regmap, reg, bulk_val, 2); +} + static int max31790_read_fan(struct device *dev, u32 attr, int channel, long *val) { - struct max31790_data *data = max31790_update_device(dev); - int sr, rpm; + struct max31790_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; + int rpm, dynamics, tach, fault; if (IS_ERR(data)) return PTR_ERR(data); switch (attr) { case hwmon_fan_input: - sr = get_tach_period(data->fan_dynamics[channel]); - rpm = RPM_FROM_REG(data->tach[channel], sr); + dynamics = read_reg_byte(regmap, MAX31790_REG_FAN_DYNAMICS(channel)); + if (dynamics < 0) + return dynamics; + + tach = read_reg_word(regmap, MAX31790_REG_TACH_COUNT(channel)); + if (tach < 0) + return tach; + + rpm = RPM_FROM_REG(tach, get_tach_period(dynamics)); *val = rpm; return 0; case hwmon_fan_target: - sr = get_tach_period(data->fan_dynamics[channel]); - rpm = RPM_FROM_REG(data->target_count[channel], sr); + dynamics = read_reg_byte(regmap, MAX31790_REG_FAN_DYNAMICS(channel)); + if (dynamics < 0) + return dynamics; + + tach = read_reg_word(regmap, MAX31790_REG_TARGET_COUNT(channel)); + if (tach < 0) + return tach; + + rpm = RPM_FROM_REG(tach, get_tach_period(dynamics)); *val = rpm; return 0; case hwmon_fan_fault: - *val = !!(data->fault_status & (1 << channel)); + if (channel > 6) + fault = read_reg_byte(regmap, MAX31790_REG_FAN_FAULT_STATUS2); + else + fault = read_reg_byte(regmap, MAX31790_REG_FAN_FAULT_STATUS1); + + if (fault < 0) + return fault; + + if (channel > 6) + *val = !!(fault & (1 << (channel - 6))); + else + *val = !!(fault & (1 << channel)); return 0; default: return -EOPNOTSUPP; @@ -191,52 +210,58 @@ static int max31790_write_fan(struct device *dev, u32 attr, int channel, long val) { struct max31790_data *data = dev_get_drvdata(dev); - struct i2c_client *client = data->client; + struct regmap *regmap = data->regmap; int target_count; int err = 0; u8 bits; int sr; - - mutex_lock(&data->update_lock); + int fan_dynamics; switch (attr) { case hwmon_fan_target: val = clamp_val(val, FAN_RPM_MIN, FAN_RPM_MAX); bits = bits_for_tach_period(val); - data->fan_dynamics[channel] = - ((data->fan_dynamics[channel] & + fan_dynamics = read_reg_byte(regmap, MAX31790_REG_FAN_DYNAMICS(channel)); + + if (fan_dynamics < 0) + return fan_dynamics; + + fan_dynamics = + ((fan_dynamics & ~MAX31790_FAN_DYN_SR_MASK) | (bits << MAX31790_FAN_DYN_SR_SHIFT)); - err = i2c_smbus_write_byte_data(client, - MAX31790_REG_FAN_DYNAMICS(channel), - data->fan_dynamics[channel]); + err = regmap_write(regmap, + MAX31790_REG_FAN_DYNAMICS(channel), + fan_dynamics); if (err < 0) break; - sr = get_tach_period(data->fan_dynamics[channel]); + sr = get_tach_period(fan_dynamics); target_count = RPM_TO_REG(val, sr); target_count = clamp_val(target_count, 0x1, 0x7FF); - data->target_count[channel] = target_count << 5; + target_count = target_count << 5; - err = i2c_smbus_write_word_swapped(client, - MAX31790_REG_TARGET_COUNT(channel), - data->target_count[channel]); + err = write_reg_word(regmap, + MAX31790_REG_TARGET_COUNT(channel), + target_count); break; default: err = -EOPNOTSUPP; break; } - mutex_unlock(&data->update_lock); - return err; } static umode_t max31790_fan_is_visible(const void *_data, u32 attr, int channel) { const struct max31790_data *data = _data; - u8 fan_config = data->fan_config[channel % NR_CHANNEL]; + struct regmap *regmap = data->regmap; + u8 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); + + if (fan_config < 0) + return 0; switch (attr) { case hwmon_fan_input: @@ -258,22 +283,29 @@ static umode_t max31790_fan_is_visible(const void *_data, u32 attr, int channel) static int max31790_read_pwm(struct device *dev, u32 attr, int channel, long *val) { - struct max31790_data *data = max31790_update_device(dev); - u8 fan_config; + struct max31790_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; + int read; if (IS_ERR(data)) return PTR_ERR(data); - fan_config = data->fan_config[channel]; - switch (attr) { case hwmon_pwm_input: - *val = data->pwm[channel] >> 8; + read = read_reg_word(regmap, MAX31790_REG_PWMOUT(channel)); + if (read < 0) + return read; + + *val = read >> 8; return 0; case hwmon_pwm_enable: - if (fan_config & MAX31790_FAN_CFG_RPM_MODE) + read = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel)); + if (read < 0) + return read; + + if (read & MAX31790_FAN_CFG_RPM_MODE) *val = 2; - else if (fan_config & MAX31790_FAN_CFG_TACH_INPUT_EN) + else if (read & MAX31790_FAN_CFG_TACH_INPUT_EN) *val = 1; else *val = 0; @@ -287,25 +319,24 @@ static int max31790_write_pwm(struct device *dev, u32 attr, int channel, long val) { struct max31790_data *data = dev_get_drvdata(dev); - struct i2c_client *client = data->client; + struct regmap *regmap = data->regmap; u8 fan_config; int err = 0; - mutex_lock(&data->update_lock); - switch (attr) { case hwmon_pwm_input: if (val < 0 || val > 255) { err = -EINVAL; break; } - data->pwm[channel] = val << 8; - err = i2c_smbus_write_word_swapped(client, - MAX31790_REG_PWMOUT(channel), - data->pwm[channel]); + err = write_reg_word(regmap, MAX31790_REG_PWMOUT(channel), val << 8); break; case hwmon_pwm_enable: - fan_config = data->fan_config[channel]; + fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); + + if (fan_config < 0) + return fan_config; + if (val == 0) { fan_config &= ~(MAX31790_FAN_CFG_TACH_INPUT_EN | MAX31790_FAN_CFG_RPM_MODE); @@ -320,25 +351,26 @@ static int max31790_write_pwm(struct device *dev, u32 attr, int channel, err = -EINVAL; break; } - data->fan_config[channel] = fan_config; - err = i2c_smbus_write_byte_data(client, - MAX31790_REG_FAN_CONFIG(channel), - fan_config); + err = regmap_write(regmap, + MAX31790_REG_FAN_CONFIG(channel), + fan_config); break; default: err = -EOPNOTSUPP; break; } - mutex_unlock(&data->update_lock); - return err; } static umode_t max31790_pwm_is_visible(const void *_data, u32 attr, int channel) { const struct max31790_data *data = _data; - u8 fan_config = data->fan_config[channel]; + struct regmap *regmap = data->regmap; + u8 fan_config = read_reg_byte(regmap, MAX31790_REG_FAN_CONFIG(channel % NR_CHANNEL)); + + if (fan_config < 0) + return 0; switch (attr) { case hwmon_pwm_input: @@ -426,35 +458,12 @@ static const struct hwmon_chip_info max31790_chip_info = { .info = max31790_info, }; -static int max31790_init_client(struct i2c_client *client, - struct max31790_data *data) -{ - int i, rv; - - for (i = 0; i < NR_CHANNEL; i++) { - rv = i2c_smbus_read_byte_data(client, - MAX31790_REG_FAN_CONFIG(i)); - if (rv < 0) - return rv; - data->fan_config[i] = rv; - - rv = i2c_smbus_read_byte_data(client, - MAX31790_REG_FAN_DYNAMICS(i)); - if (rv < 0) - return rv; - data->fan_dynamics[i] = rv; - } - - return 0; -} - static int max31790_probe(struct i2c_client *client) { struct i2c_adapter *adapter = client->adapter; struct device *dev = &client->dev; struct max31790_data *data; struct device *hwmon_dev; - int err; if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA)) @@ -464,15 +473,12 @@ static int max31790_probe(struct i2c_client *client) if (!data) return -ENOMEM; - data->client = client; - mutex_init(&data->update_lock); + data->regmap = devm_regmap_init_i2c(client, &max31790_regmap_config); - /* - * Initialize the max31790 chip - */ - err = max31790_init_client(client, data); - if (err) - return err; + if (IS_ERR(data->regmap)) { + dev_err(dev, "failed to allocate register map\n"); + return PTR_ERR(data->regmap); + } hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, data,
Converting the driver to use regmap makes it more generic. It also makes it a lot easier to debug through debugfs. Signed-off-by: Václav Kubernát <kubernat@cesnet.cz> --- drivers/hwmon/Kconfig | 1 + drivers/hwmon/max31790.c | 318 ++++++++++++++++++++------------------- 2 files changed, 163 insertions(+), 156 deletions(-)