On Thu, 2024-05-23 at 07:19 -0700, Guenter Roeck wrote: > On Thu, May 23, 2024 at 03:10:56PM +0300, Radu Sabau wrote: > > Add support for PEC by attaching PEC attribute to the i2c device. > > Add pec_store and pec_show function for accesing the "pec" file. > > > > Signed-off-by: Radu Sabau <radu.sabau@xxxxxxxxxx> > > --- > > Change log missing. > > > Documentation/hwmon/max31827.rst | 13 +++++-- > > drivers/hwmon/max31827.c | 64 ++++++++++++++++++++++++++++++++ > > 2 files changed, 74 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/hwmon/max31827.rst b/Documentation/hwmon/max31827.rst > > index 44ab9dc064cb..9c11a9518c67 100644 > > --- a/Documentation/hwmon/max31827.rst > > +++ b/Documentation/hwmon/max31827.rst > > @@ -131,7 +131,14 @@ 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 Support > > +----------- > > + > > +When reading a register value, the PEC byte is computed and sent by the chip. > > + > > +PEC on word data transaction respresents a signifcant increase in bandwitdh > > +usage (+33% for both write and reads) in normal conditions. > > > > -PEC is not implemented. > > +Since this operation implies there will be an extra delay to each > > +transaction, PEC can be disabled or enabled through sysfs. > > +Just write 1 to the "pec" file for enabling PEC and 0 for disabling it. > > diff --git a/drivers/hwmon/max31827.c b/drivers/hwmon/max31827.c > > index f8a13b30f100..e86f8890ee72 100644 > > --- a/drivers/hwmon/max31827.c > > +++ b/drivers/hwmon/max31827.c > > @@ -24,6 +24,7 @@ > > > > #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) > > @@ -475,6 +476,54 @@ static ssize_t temp1_resolution_store(struct device *dev, > > > > static DEVICE_ATTR_RW(temp1_resolution); > > > > +static ssize_t pec_show(struct device *dev, struct device_attribute *devattr, > > + char *buf) > > +{ > > + struct i2c_client *client = to_i2c_client(dev); > > + > > + 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 = to_i2c_client(dev); > > + unsigned int val; > > + int err; > > + > > + err = kstrtouint(buf, 10, &val); > > + if (err < 0) > > + return err; > > + > > + switch (val) { > > + case 0: > > + err = regmap_update_bits(st->regmap, MAX31827_CONFIGURATION_REG, > > + MAX31827_CONFIGURATION_PEC_EN_MASK, > > + val); > > While correct, this is misleading. Should write 0. > > > + if (err) > > + return err; > > + > > + client->flags &= ~I2C_CLIENT_PEC; > > + break; > > + case 1: > > + err = regmap_update_bits(st->regmap, MAX31827_CONFIGURATION_REG, > > + MAX31827_CONFIGURATION_PEC_EN_MASK, > > + val); > > This is wrong. s/val/MAX31827_CONFIGURATION_PEC_EN_MASK/ > > Then, maybe use regmap_set_bits()... - Nuno Sá