Removed regmap and used my functions to read, write and update bits. In these functions i2c_smbus_ helper functions are used. These check if there were any PEC errors during read. In the write function, if PEC is enabled, I check for PEC Error bit, to see if there were any errors. Signed-off-by: Daniel Matyas <daniel.matyas@xxxxxxxxxx> --- Documentation/hwmon/max31827.rst | 14 +- drivers/hwmon/max31827.c | 219 +++++++++++++++++++++++-------- 2 files changed, 171 insertions(+), 62 deletions(-) diff --git a/Documentation/hwmon/max31827.rst b/Documentation/hwmon/max31827.rst index 44ab9dc064cb..ecbc1ddba6a7 100644 --- a/Documentation/hwmon/max31827.rst +++ b/Documentation/hwmon/max31827.rst @@ -131,7 +131,13 @@ The Fault Queue bits select how many consecutive temperature faults must occur before overtemperature or undertemperature faults are indicated in the corresponding status bits. -Notes ------ - -PEC is not implemented. +PEC (packet error checking) can be enabled from the "pec" device attribute. +If PEC is enabled, a PEC byte is appended to the end of each message transfer. +This is a CRC-8 byte that is calculated on all of the message bytes (including +the address/read/write byte). The last device to transmit a data byte also +transmits the PEC byte. The master transmits the PEC byte after a write +transaction, and the MAX31827 transmits the PEC byte after a read transaction. + +The read PEC error is handled inside the i2c_smbus_read_word_swapped() function. +To check if the write had any PEC error a read is performed on the configuration +register, to check the PEC Error bit. \ No newline at end of file diff --git a/drivers/hwmon/max31827.c b/drivers/hwmon/max31827.c index 71ad3934dfb6..db93492193bd 100644 --- a/drivers/hwmon/max31827.c +++ b/drivers/hwmon/max31827.c @@ -11,8 +11,8 @@ #include <linux/hwmon.h> #include <linux/i2c.h> #include <linux/mutex.h> -#include <linux/regmap.h> #include <linux/regulator/consumer.h> +#include <linux/hwmon-sysfs.h> #include <linux/of_device.h> #define MAX31827_T_REG 0x0 @@ -24,11 +24,13 @@ #define MAX31827_CONFIGURATION_1SHOT_MASK BIT(0) #define MAX31827_CONFIGURATION_CNV_RATE_MASK GENMASK(3, 1) +#define MAX31827_CONFIGURATION_PEC_EN_MASK BIT(4) #define MAX31827_CONFIGURATION_TIMEOUT_MASK BIT(5) #define MAX31827_CONFIGURATION_RESOLUTION_MASK GENMASK(7, 6) #define MAX31827_CONFIGURATION_ALRM_POL_MASK BIT(8) #define MAX31827_CONFIGURATION_COMP_INT_MASK BIT(9) #define MAX31827_CONFIGURATION_FLT_Q_MASK GENMASK(11, 10) +#define MAX31827_CONFIGURATION_PEC_ERR_MASK BIT(13) #define MAX31827_CONFIGURATION_U_TEMP_STAT_MASK BIT(14) #define MAX31827_CONFIGURATION_O_TEMP_STAT_MASK BIT(15) @@ -94,23 +96,67 @@ struct max31827_state { * Prevent simultaneous access to the i2c client. */ struct mutex lock; - struct regmap *regmap; bool enable; unsigned int resolution; unsigned int update_interval; + struct i2c_client *client; }; -static const struct regmap_config max31827_regmap = { - .reg_bits = 8, - .val_bits = 16, - .max_register = 0xA, -}; +static int max31827_reg_read(struct i2c_client *client, u8 reg, u16 *val) +{ + u16 tmp = i2c_smbus_read_word_swapped(client, reg); + + if (tmp < 0) + return tmp; + + *val = tmp; + return 0; +} + +static int max31827_reg_write(struct i2c_client *client, u8 reg, u16 val) +{ + u16 cfg; + int ret; + + ret = i2c_smbus_write_word_swapped(client, reg, val); + if (ret) + return ret; + + // If PEC is not enabled, return with success + if (!(client->flags & I2C_CLIENT_PEC)) + return 0; + + ret = max31827_reg_read(client, MAX31827_CONFIGURATION_REG, &cfg); + if (ret) + return ret; + + if (cfg & MAX31827_CONFIGURATION_PEC_ERR_MASK) + return -EBADMSG; + + return 0; +} + +static int max31827_update_bits(struct i2c_client *client, u8 reg, + u16 mask, u16 val) +{ + u16 tmp; + int ret; + + ret = max31827_reg_read(client, reg, &tmp); + if (ret) + return ret; + + tmp = (tmp & ~mask) | (val & mask); + ret = max31827_reg_write(client, reg, tmp); + + return ret; +} static int shutdown_write(struct max31827_state *st, unsigned int reg, unsigned int mask, unsigned int val) { - unsigned int cfg; - unsigned int cnv_rate; + u16 cfg; + u16 cnv_rate; int ret; /* @@ -125,34 +171,34 @@ static int shutdown_write(struct max31827_state *st, unsigned int reg, if (!st->enable) { if (!mask) - ret = regmap_write(st->regmap, reg, val); + ret = max31827_reg_write(st->client, reg, val); else - ret = regmap_update_bits(st->regmap, reg, mask, val); + ret = max31827_update_bits(st->client, reg, mask, val); goto unlock; } - ret = regmap_read(st->regmap, MAX31827_CONFIGURATION_REG, &cfg); + ret = max31827_reg_read(st->client, MAX31827_CONFIGURATION_REG, &cfg); if (ret) goto unlock; cnv_rate = MAX31827_CONFIGURATION_CNV_RATE_MASK & cfg; cfg = cfg & ~(MAX31827_CONFIGURATION_1SHOT_MASK | MAX31827_CONFIGURATION_CNV_RATE_MASK); - ret = regmap_write(st->regmap, MAX31827_CONFIGURATION_REG, cfg); + ret = max31827_reg_write(st->client, MAX31827_CONFIGURATION_REG, cfg); if (ret) goto unlock; if (!mask) - ret = regmap_write(st->regmap, reg, val); + ret = max31827_reg_write(st->client, reg, val); else - ret = regmap_update_bits(st->regmap, reg, mask, val); + ret = max31827_update_bits(st->client, reg, mask, val); if (ret) goto unlock; - ret = regmap_update_bits(st->regmap, MAX31827_CONFIGURATION_REG, - MAX31827_CONFIGURATION_CNV_RATE_MASK, - cnv_rate); + ret = max31827_update_bits(st->client, MAX31827_CONFIGURATION_REG, + MAX31827_CONFIGURATION_CNV_RATE_MASK, + cnv_rate); unlock: mutex_unlock(&st->lock); @@ -198,15 +244,16 @@ static int max31827_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel, long *val) { struct max31827_state *st = dev_get_drvdata(dev); - unsigned int uval; + u16 uval; int ret = 0; switch (type) { case hwmon_temp: switch (attr) { case hwmon_temp_enable: - ret = regmap_read(st->regmap, - MAX31827_CONFIGURATION_REG, &uval); + ret = max31827_reg_read(st->client, + MAX31827_CONFIGURATION_REG, + &uval); if (ret) break; @@ -226,10 +273,10 @@ static int max31827_read(struct device *dev, enum hwmon_sensor_types type, * be changed during the conversion process. */ - ret = regmap_update_bits(st->regmap, - MAX31827_CONFIGURATION_REG, - MAX31827_CONFIGURATION_1SHOT_MASK, - 1); + ret = max31827_update_bits(st->client, + MAX31827_CONFIGURATION_REG, + MAX31827_CONFIGURATION_1SHOT_MASK, + 1); if (ret) { mutex_unlock(&st->lock); return ret; @@ -246,7 +293,8 @@ static int max31827_read(struct device *dev, enum hwmon_sensor_types type, st->update_interval == 125) usleep_range(15000, 20000); - ret = regmap_read(st->regmap, MAX31827_T_REG, &uval); + ret = max31827_reg_read(st->client, MAX31827_T_REG, + &uval); mutex_unlock(&st->lock); @@ -257,23 +305,26 @@ static int max31827_read(struct device *dev, enum hwmon_sensor_types type, break; case hwmon_temp_max: - ret = regmap_read(st->regmap, MAX31827_TH_REG, &uval); + ret = max31827_reg_read(st->client, MAX31827_TH_REG, + &uval); if (ret) break; *val = MAX31827_16_BIT_TO_M_DGR(uval); break; case hwmon_temp_max_hyst: - ret = regmap_read(st->regmap, MAX31827_TH_HYST_REG, - &uval); + ret = max31827_reg_read(st->client, + MAX31827_TH_HYST_REG, + &uval); if (ret) break; *val = MAX31827_16_BIT_TO_M_DGR(uval); break; case hwmon_temp_max_alarm: - ret = regmap_read(st->regmap, - MAX31827_CONFIGURATION_REG, &uval); + ret = max31827_reg_read(st->client, + MAX31827_CONFIGURATION_REG, + &uval); if (ret) break; @@ -281,23 +332,25 @@ static int max31827_read(struct device *dev, enum hwmon_sensor_types type, uval); break; case hwmon_temp_min: - ret = regmap_read(st->regmap, MAX31827_TL_REG, &uval); + ret = max31827_reg_read(st->client, MAX31827_TL_REG, + &uval); if (ret) break; *val = MAX31827_16_BIT_TO_M_DGR(uval); break; case hwmon_temp_min_hyst: - ret = regmap_read(st->regmap, MAX31827_TL_HYST_REG, - &uval); + ret = max31827_reg_read(st->client, MAX31827_TL_HYST_REG, + &uval); if (ret) break; *val = MAX31827_16_BIT_TO_M_DGR(uval); break; case hwmon_temp_min_alarm: - ret = regmap_read(st->regmap, - MAX31827_CONFIGURATION_REG, &uval); + ret = max31827_reg_read(st->client, + MAX31827_CONFIGURATION_REG, + &uval); if (ret) break; @@ -313,8 +366,9 @@ static int max31827_read(struct device *dev, enum hwmon_sensor_types type, case hwmon_chip: if (attr == hwmon_chip_update_interval) { - ret = regmap_read(st->regmap, - MAX31827_CONFIGURATION_REG, &uval); + ret = max31827_reg_read(st->client, + MAX31827_CONFIGURATION_REG, + &uval); if (ret) break; @@ -355,11 +409,11 @@ static int max31827_write(struct device *dev, enum hwmon_sensor_types type, st->enable = val; - ret = regmap_update_bits(st->regmap, - MAX31827_CONFIGURATION_REG, - MAX31827_CONFIGURATION_1SHOT_MASK | - MAX31827_CONFIGURATION_CNV_RATE_MASK, - MAX31827_DEVICE_ENABLE(val)); + ret = max31827_update_bits(st->client, + MAX31827_CONFIGURATION_REG, + MAX31827_CONFIGURATION_1SHOT_MASK | + MAX31827_CONFIGURATION_CNV_RATE_MASK, + MAX31827_DEVICE_ENABLE(val)); mutex_unlock(&st->lock); @@ -402,10 +456,10 @@ static int max31827_write(struct device *dev, enum hwmon_sensor_types type, res = FIELD_PREP(MAX31827_CONFIGURATION_CNV_RATE_MASK, res); - ret = regmap_update_bits(st->regmap, - MAX31827_CONFIGURATION_REG, - MAX31827_CONFIGURATION_CNV_RATE_MASK, - res); + ret = max31827_update_bits(st->client, + MAX31827_CONFIGURATION_REG, + MAX31827_CONFIGURATION_CNV_RATE_MASK, + res); if (ret) return ret; @@ -425,10 +479,10 @@ static ssize_t temp1_resolution_show(struct device *dev, char *buf) { struct max31827_state *st = dev_get_drvdata(dev); - unsigned int val; + u16 val; int ret; - ret = regmap_read(st->regmap, MAX31827_CONFIGURATION_REG, &val); + ret = max31827_reg_read(st->client, MAX31827_CONFIGURATION_REG, &val); if (ret) return ret; @@ -473,10 +527,63 @@ static ssize_t temp1_resolution_store(struct device *dev, return ret ? ret : count; } +static ssize_t pec_show(struct device *dev, struct device_attribute *devattr, + char *buf) +{ + struct max31827_state *st = dev_get_drvdata(dev); + struct i2c_client *client = st->client; + + return scnprintf(buf, PAGE_SIZE, "%d\n", !!(client->flags & I2C_CLIENT_PEC)); +} + +static ssize_t pec_store(struct device *dev, struct device_attribute *devattr, + const char *buf, size_t count) +{ + struct max31827_state *st = dev_get_drvdata(dev); + struct i2c_client *client = st->client; + unsigned int val; + u16 val2; + int err; + + err = kstrtouint(buf, 10, &val); + if (err < 0) + return err; + + val2 = FIELD_PREP(MAX31827_CONFIGURATION_PEC_EN_MASK, val); + + if (err) + return err; + + switch (val) { + case 0: + client->flags &= ~I2C_CLIENT_PEC; + err = max31827_update_bits(client, MAX31827_CONFIGURATION_REG, + MAX31827_CONFIGURATION_PEC_EN_MASK, + val2); + if (err) + return err; + break; + case 1: + err = max31827_update_bits(client, MAX31827_CONFIGURATION_REG, + MAX31827_CONFIGURATION_PEC_EN_MASK, + val2); + if (err) + return err; + client->flags |= I2C_CLIENT_PEC; + break; + default: + return -EINVAL; + } + + return count; +} + static DEVICE_ATTR_RW(temp1_resolution); +static DEVICE_ATTR_RW(pec); static struct attribute *max31827_attrs[] = { &dev_attr_temp1_resolution.attr, + &dev_attr_pec.attr, NULL }; ATTRIBUTE_GROUPS(max31827); @@ -489,9 +596,9 @@ static const struct i2c_device_id max31827_i2c_ids[] = { }; MODULE_DEVICE_TABLE(i2c, max31827_i2c_ids); -static int max31827_init_client(struct max31827_state *st, - struct device *dev) +static int max31827_init_client(struct max31827_state *st) { + struct device *dev = &st->client->dev; struct fwnode_handle *fwnode; unsigned int res = 0; u32 data, lsb_idx; @@ -575,7 +682,7 @@ static int max31827_init_client(struct max31827_state *st, } } - return regmap_write(st->regmap, MAX31827_CONFIGURATION_REG, res); + return max31827_reg_write(st->client, MAX31827_CONFIGURATION_REG, res); } static const struct hwmon_channel_info *max31827_info[] = { @@ -613,17 +720,13 @@ static int max31827_probe(struct i2c_client *client) return -ENOMEM; mutex_init(&st->lock); - - st->regmap = devm_regmap_init_i2c(client, &max31827_regmap); - if (IS_ERR(st->regmap)) - return dev_err_probe(dev, PTR_ERR(st->regmap), - "Failed to allocate regmap.\n"); + st->client = client; err = devm_regulator_get_enable(dev, "vref"); if (err) return dev_err_probe(dev, err, "failed to enable regulator\n"); - err = max31827_init_client(st, dev); + err = max31827_init_client(st); if (err) return err; -- 2.34.1