diff mbox series

[v4] drivers: hwmon: max31827: Add PEC support

Message ID 20240527092947.4370-1-radu.sabau@analog.com (mailing list archive)
State Changes Requested
Headers show
Series [v4] drivers: hwmon: max31827: Add PEC support | expand

Commit Message

Radu Sabau May 27, 2024, 9:29 a.m. UTC
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 <radu.sabau@analog.com>
---
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(-)

Comments

Guenter Roeck May 29, 2024, 6:01 p.m. UTC | #1
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 <radu.sabau@analog.com>
> ---

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
diff mbox series

Patch

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);