Re: [PATCH v2 1/5] hwmon: (max31790) Rework to use regmap
Hello, I'm uploading a new version of my patches on max31790. This is a "v3" patch, but I have mistakenly tagged it as "v2". Hopefully, this is not a big issue. Changes: - I have reintroduced locking. However, I'm not sure if it's enough, I think, locking needs to happen even when reading, but I'm not sure - fan_config and fan_dynamics are now saved locally - I have fixed formatting issues Václav út 13. 4. 2021 v 5:02 odesílatel Václav Kubernát napsal: > > 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 > --- > drivers/hwmon/Kconfig| 1 + > drivers/hwmon/max31790.c | 254 --- > 2 files changed, 133 insertions(+), 122 deletions(-) > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 1ecf697d8d99..9f11d036c316 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1095,6 +1095,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 2c6b333a28e9..e3765cea 100644 > --- a/drivers/hwmon/max31790.c > +++ b/drivers/hwmon/max31790.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > #include > > /* MAX31790 registers */ > @@ -46,92 +47,53 @@ > > #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 = _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 = _wr_table, > + .volatile_table = _volatile_table > +}; > + > /* > * Client data (each client gets its own) > */ > struct max31790_data { > - struct i2c_client *client; > + struct regmap *regmap; > + > 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]; > }; > > -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(>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 > -
[PATCH v2 1/5] hwmon: (max31790) Rework to use regmap
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 --- drivers/hwmon/Kconfig| 1 + drivers/hwmon/max31790.c | 254 --- 2 files changed, 133 insertions(+), 122 deletions(-) diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 1ecf697d8d99..9f11d036c316 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1095,6 +1095,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 2c6b333a28e9..e3765cea 100644 --- a/drivers/hwmon/max31790.c +++ b/drivers/hwmon/max31790.c @@ -12,6 +12,7 @@ #include #include #include +#include #include /* MAX31790 registers */ @@ -46,92 +47,53 @@ #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 = _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 = _wr_table, + .volatile_table = _volatile_table +}; + /* * Client data (each client gets its own) */ struct max31790_data { - struct i2c_client *client; + struct regmap *regmap; + 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]; }; -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(>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) -
Re: [PATCH v2 1/5] hwmon: (max31790) Rework to use regmap
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 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 > > 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 > > #include > > #include > > +#include > > #include > > > > /* MAX31790 registers */ > > @@ -46,92 +47,49 @@ > > > > #define NR_CHANNEL 6 > > > > +#define MAX31790_REG_USER_BYTE_670x67 > > + > > +#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 = _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 = _wr_table, > > + .volatile_table = _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(>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 =
Re: [PATCH v2 1/5] hwmon: (max31790) Rework to use regmap
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 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 > Reported-by: Dan Carpenter > > 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)); >
Re: [PATCH v2 1/5] hwmon: (max31790) Rework to use regmap
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 Reported-by: Dan Carpenter 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 &
Re: [PATCH v2 1/5] hwmon: (max31790) Rework to use regmap
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
[PATCH v2 1/5] hwmon: (max31790) Rework to use regmap
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 --- 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 #include #include +#include #include /* 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 = _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 = _wr_table, + .volatile_table = _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(>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) -