From: Guenter Roeck <[email protected]>
Several hardware monitoring chips optionally support Packet Error Checking
(PEC). For some chips, PEC support can be enabled simply by setting
I2C_CLIENT_PEC in the i2c client data structure. Others require chip
specific code to enable or disable PEC support.
Introduce hwmon_chip_pec and HWMON_C_PEC to simplify adding configurable
PEC support for hardware monitoring drivers. A driver can set HWMON_C_PEC
in its chip information data to indicate PEC support. If a chip requires
chip specific code to enable or disable PEC support, the driver only needs
to implement support for the hwmon_chip_pec attribute to its write
function.
The hardware monitoring core does not depend on the I2C subsystem after
this change. However, the I2C subsystem needs to be reachable. This
requires a new HWMON dependency to ensure that HWMON can only be built
as module if I2C is built as module. This should not make a practical
difference.
Cc: Radu Sabau <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
Acked-by: Nuno Sa <[email protected]>
---
drivers/hwmon/Kconfig | 1 +
drivers/hwmon/hwmon.c | 136 +++++++++++++++++++++++++++++++++++++-----
include/linux/hwmon.h | 2 +
3 files changed, 123 insertions(+), 16 deletions(-)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index e14ae18a973b..7f384a2494c9 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -6,6 +6,7 @@
menuconfig HWMON
tristate "Hardware Monitoring support"
depends on HAS_IOMEM
+ depends on I2C || I2C=n
default y
help
Hardware monitoring devices let you monitor the hardware health
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 3b259c425ab7..1fdea8b1ec91 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -14,6 +14,7 @@
#include <linux/err.h>
#include <linux/gfp.h>
#include <linux/hwmon.h>
+#include <linux/i2c.h>
#include <linux/idr.h>
#include <linux/kstrtox.h>
#include <linux/list.h>
@@ -309,6 +310,103 @@ static int hwmon_attr_base(enum hwmon_sensor_types type)
return 1;
}
+/*
+ * PEC support
+ *
+ * The 'pec' attribute is attached to I2C client devices. It is only provided
+ * if the i2c controller supports PEC.
+ *
+ * The mutex ensures that PEC configuration between i2c device and the hardware
+ * is consistent. Use a single mutex because attribute writes are supposed to be
+ * rare, and maintaining a separate mutex for each hardware monitoring device
+ * would add substantial complexity to the driver for little if any gain.
+ *
+ * The hardware monitoring device is identified as child of the i2c client
+ * device. This assumes that only a single hardware monitoring device is
+ * attached to an i2c client device.
+ */
+
+static DEFINE_MUTEX(hwmon_pec_mutex);
+
+static int hwmon_match_device(struct device *dev, void *data)
+{
+ return dev->class == &hwmon_class;
+}
+
+static ssize_t pec_show(struct device *dev, struct device_attribute *dummy,
+ char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+
+ return sprintf(buf, "%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 i2c_client *client = to_i2c_client(dev);
+ struct hwmon_device *hwdev;
+ struct device *hdev;
+ bool val;
+ int err;
+
+ err = kstrtobool(buf, &val);
+ if (err < 0)
+ return err;
+
+ hdev = device_find_child(dev, NULL, hwmon_match_device);
+ if (!hdev)
+ return -ENODEV;
+
+ mutex_lock(&hwmon_pec_mutex);
+
+ /*
+ * If there is no write function, we assume that chip specific
+ * handling is not required.
+ */
+ hwdev = to_hwmon_device(hdev);
+ if (hwdev->chip->ops->write) {
+ err = hwdev->chip->ops->write(hdev, hwmon_chip, hwmon_chip_pec, 0, val);
+ if (err && err != -EOPNOTSUPP)
+ goto unlock;
+ }
+
+ if (!val)
+ client->flags &= ~I2C_CLIENT_PEC;
+ else
+ client->flags |= I2C_CLIENT_PEC;
+
+ err = count;
+unlock:
+ mutex_unlock(&hwmon_pec_mutex);
+ put_device(hdev);
+
+ return err;
+}
+
+static DEVICE_ATTR_RW(pec);
+
+static void hwmon_remove_pec(void *dev)
+{
+ device_remove_file(dev, &dev_attr_pec);
+}
+
+static int hwmon_pec_register(struct device *hdev)
+{
+ struct i2c_client *client = i2c_verify_client(hdev->parent);
+ int err;
+
+ if (!client ||
+ !i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_PEC))
+ return 0;
+
+ err = device_create_file(&client->dev, &dev_attr_pec);
+ if (err)
+ return err;
+
+ return devm_add_action_or_reset(hdev, hwmon_remove_pec, &client->dev);
+}
+
/* sysfs attribute management */
static ssize_t hwmon_attr_show(struct device *dev,
@@ -397,10 +495,6 @@ static struct attribute *hwmon_genattr(const void *drvdata,
const char *name;
bool is_string = is_string_attr(type, attr);
- /* The attribute is invisible if there is no template string */
- if (!template)
- return ERR_PTR(-ENOENT);
-
mode = ops->is_visible(drvdata, type, attr, index);
if (!mode)
return ERR_PTR(-ENOENT);
@@ -712,8 +806,8 @@ static int hwmon_genattrs(const void *drvdata,
attr = __ffs(attr_mask);
attr_mask &= ~BIT(attr);
- if (attr >= template_size)
- return -EINVAL;
+ if (attr >= template_size || !templates[attr])
+ continue; /* attribute is invisible */
a = hwmon_genattr(drvdata, info->type, attr, i,
templates[attr], ops);
if (IS_ERR(a)) {
@@ -849,16 +943,26 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
INIT_LIST_HEAD(&hwdev->tzdata);
if (hdev->of_node && chip && chip->ops->read &&
- chip->info[0]->type == hwmon_chip &&
- (chip->info[0]->config[0] & HWMON_C_REGISTER_TZ)) {
- err = hwmon_thermal_register_sensors(hdev);
- if (err) {
- device_unregister(hdev);
- /*
- * Don't worry about hwdev; hwmon_dev_release(), called
- * from device_unregister(), will free it.
- */
- goto ida_remove;
+ chip->info[0]->type == hwmon_chip) {
+ u32 config = chip->info[0]->config[0];
+
+ if (config & HWMON_C_REGISTER_TZ) {
+ err = hwmon_thermal_register_sensors(hdev);
+ if (err) {
+ device_unregister(hdev);
+ /*
+ * Don't worry about hwdev; hwmon_dev_release(),
+ * called from device_unregister(), will free it.
+ */
+ goto ida_remove;
+ }
+ }
+ if (config & HWMON_C_PEC) {
+ err = hwmon_pec_register(hdev);
+ if (err) {
+ device_unregister(hdev);
+ goto ida_remove;
+ }
}
}
diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
index edf96f249eb5..e94314760aab 100644
--- a/include/linux/hwmon.h
+++ b/include/linux/hwmon.h
@@ -45,6 +45,7 @@ enum hwmon_chip_attributes {
hwmon_chip_power_samples,
hwmon_chip_temp_samples,
hwmon_chip_beep_enable,
+ hwmon_chip_pec,
};
#define HWMON_C_TEMP_RESET_HISTORY BIT(hwmon_chip_temp_reset_history)
@@ -60,6 +61,7 @@ enum hwmon_chip_attributes {
#define HWMON_C_POWER_SAMPLES BIT(hwmon_chip_power_samples)
#define HWMON_C_TEMP_SAMPLES BIT(hwmon_chip_temp_samples)
#define HWMON_C_BEEP_ENABLE BIT(hwmon_chip_beep_enable)
+#define HWMON_C_PEC BIT(hwmon_chip_pec)
enum hwmon_temp_attributes {
hwmon_temp_enable,
--
2.34.1
Add support for PEC by configuring the chip accordingly to the hwmon core PEC
attribute handling
Handle hwmon_chip_pec attribute writing in the max31827_write in the
hwmon_chip type switch case, approaching the same code structure
as for temp writing.
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.
v5:
*Adapt driver to the new hwmon PEC attribute handling from the hwmon core.
v6:
*Apply patch containing hwmon core code and rebase driver PEC patch on top
of it.
*Handle hwmon_chip_pec attribute write in the max31827_write function, the same
way temp writes are handled, therefore remove the max31827_chip_write function
*Fix typos.
---
Documentation/hwmon/max31827.rst | 13 ++++++++++---
drivers/hwmon/max31827.c | 18 ++++++++++++------
2 files changed, 22 insertions(+), 9 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..4d89b6a7060b 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)
@@ -382,7 +383,8 @@ static int max31827_write(struct device *dev, enum hwmon_sensor_types type,
}
case hwmon_chip:
- if (attr == hwmon_chip_update_interval) {
+ switch (attr) {
+ case hwmon_chip_update_interval:
if (!st->enable)
return -EINVAL;
@@ -410,14 +412,18 @@ static int max31827_write(struct device *dev, enum hwmon_sensor_types type,
return ret;
st->update_interval = val;
- }
- break;
+ return 0;
+ case hwmon_chip_pec:
+ return regmap_update_bits(st->regmap, MAX31827_CONFIGURATION_REG,
+ MAX31827_CONFIGURATION_PEC_EN_MASK,
+ val ? MAX31827_CONFIGURATION_PEC_EN_MASK : 0);
+ default:
+ return -EOPNOTSUPP;
+ }
default:
return -EOPNOTSUPP;
}
-
- return 0;
}
static ssize_t temp1_resolution_show(struct device *dev,
@@ -583,7 +589,7 @@ static const struct hwmon_channel_info *max31827_info[] = {
HWMON_T_MIN_HYST | HWMON_T_MIN_ALARM |
HWMON_T_MAX | HWMON_T_MAX_HYST |
HWMON_T_MAX_ALARM),
- HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL),
+ HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL | HWMON_C_PEC),
NULL,
};
--
2.34.1
On Fri, 2024-05-31 at 11:46 +0300, Radu Sabau wrote:
> Add support for PEC by configuring the chip accordingly to the hwmon core PEC
> attribute handling
> Handle hwmon_chip_pec attribute writing in the max31827_write in the
> hwmon_chip type switch case, approaching the same code structure
> as for temp writing.
>
> Signed-off-by: Radu Sabau <[email protected]>
> ---
Reviewed-by: Nuno Sa <[email protected]>
- Nuno Sá
>
On Fri, May 31, 2024 at 11:46:44AM +0300, Radu Sabau wrote:
> Add support for PEC by configuring the chip accordingly to the hwmon
> core PEC attribute handling.
>
> Handle hwmon_chip_pec attribute writing in the max31827_write in the
> hwmon_chip type switch case, using the same code structure as for
> writing temperature limits.
>
> Signed-off-by: Radu Sabau <[email protected]>
> Reviewed-by: Nuno Sa <[email protected]>
Applied to hwmon-next.
Thanks,
Guenter