2024-05-27 09:30:40

by Radu Sabau

[permalink] [raw]
Subject: [PATCH v4] drivers: hwmon: max31827: Add PEC support

Add support for PEC by attaching PEC attribute to the i2c device.
Add pec_store and pec_show function for accessing the "pec" file.

Signed-off-by: Radu Sabau <[email protected]>
---
Change log:
v2:
*Rebase on top of v6.9
*Attach pec attribute only to i2c device
*Fix bug to attach pec attribute to i2c device if the device supports it.
v3:
*Use only one variable to write PEC_EN bit in configuration register
*Use regmap_set_bits to set PEC_EN bit when requested instead of
regmap_update_bits.
*Fix typo in commit message.
v4:
*Use regmap_clear_bits to clear PEC_EN bit when requested instead of
regmap_update_bits.
---
Documentation/hwmon/max31827.rst | 13 +++++--
drivers/hwmon/max31827.c | 62 ++++++++++++++++++++++++++++++++
2 files changed, 72 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..c120032582ac 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,52 @@ 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_clear_bits(st->regmap, MAX31827_CONFIGURATION_REG,
+ MAX31827_CONFIGURATION_PEC_EN_MASK);
+ if (err)
+ return err;
+
+ client->flags &= ~I2C_CLIENT_PEC;
+ break;
+ case 1:
+ err = regmap_set_bits(st->regmap, MAX31827_CONFIGURATION_REG,
+ MAX31827_CONFIGURATION_PEC_EN_MASK);
+ if (err)
+ return err;
+
+ client->flags |= I2C_CLIENT_PEC;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(pec);
+
static struct attribute *max31827_attrs[] = {
&dev_attr_temp1_resolution.attr,
NULL
@@ -578,6 +625,11 @@ static int max31827_init_client(struct max31827_state *st,
return regmap_write(st->regmap, MAX31827_CONFIGURATION_REG, res);
}

+static void max31827_remove_pec(void *dev)
+{
+ device_remove_file(dev, &dev_attr_pec);
+}
+
static const struct hwmon_channel_info *max31827_info[] = {
HWMON_CHANNEL_INFO(temp, HWMON_T_ENABLE | HWMON_T_INPUT | HWMON_T_MIN |
HWMON_T_MIN_HYST | HWMON_T_MIN_ALARM |
@@ -627,6 +679,16 @@ static int max31827_probe(struct i2c_client *client)
if (err)
return err;

+ if (i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_PEC)) {
+ err = device_create_file(dev, &dev_attr_pec);
+ if (err)
+ return err;
+
+ err = devm_add_action_or_reset(dev, max31827_remove_pec, dev);
+ if (err)
+ return err;
+ }
+
hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, st,
&max31827_chip_info,
max31827_groups);
--
2.34.1



2024-05-29 18:01:33

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v4] drivers: hwmon: max31827: Add PEC support

On 5/27/24 02:29, Radu Sabau wrote:
> Add support for PEC by attaching PEC attribute to the i2c device.
> Add pec_store and pec_show function for accessing the "pec" file.
>
> Signed-off-by: Radu Sabau <[email protected]>
> ---

Sorry for the trouble, but I decided to add PEC support to the
hardware monitoring code. With those changes, the hwmon core creates
the attribute and handles i2c client configuration. The driver only
needs to configure the chip.

I'll copy you on the patches introducing and using this functionality.

Please apply the patch introducing the core changes to your system
and rebase this patch on top of it.

Thanks,
Guenter