* [PATCH 1/2] hwmon: Add PEC attribute support to hardware monitoring core
2024-05-29 18:01 [PATCH 0/2] hwmon: Add PEC attribute support to hardware monitoring core Guenter Roeck
@ 2024-05-29 18:01 ` Guenter Roeck
2024-05-30 6:37 ` Nuno Sá
2024-05-29 18:01 ` [PATCH 2/2] hwmon: (lm90) Convert to use PEC support from hwmon core Guenter Roeck
1 sibling, 1 reply; 7+ messages in thread
From: Guenter Roeck @ 2024-05-29 18:01 UTC (permalink / raw)
To: linux-hwmon; +Cc: Radu Sabau, Guenter Roeck
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 <radu.sabau@analog.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
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.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/2] hwmon: (lm90) Convert to use PEC support from hwmon core
2024-05-29 18:01 [PATCH 0/2] hwmon: Add PEC attribute support to hardware monitoring core Guenter Roeck
2024-05-29 18:01 ` [PATCH 1/2] " Guenter Roeck
@ 2024-05-29 18:01 ` Guenter Roeck
2024-05-30 6:38 ` Nuno Sá
1 sibling, 1 reply; 7+ messages in thread
From: Guenter Roeck @ 2024-05-29 18:01 UTC (permalink / raw)
To: linux-hwmon; +Cc: Radu Sabau, Guenter Roeck
Replace driver specific PEC handling code with hardware monitoring core
functionality.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/lm90.c | 56 ++------------------------------------------
1 file changed, 2 insertions(+), 54 deletions(-)
diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index e0d7454a301c..40d9e21b528c 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -1270,42 +1270,6 @@ static int lm90_update_device(struct device *dev)
return 0;
}
-/* pec used for devices with PEC support */
-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 *dummy,
- const char *buf, size_t count)
-{
- struct i2c_client *client = to_i2c_client(dev);
- long val;
- int err;
-
- err = kstrtol(buf, 10, &val);
- if (err < 0)
- return err;
-
- switch (val) {
- case 0:
- client->flags &= ~I2C_CLIENT_PEC;
- break;
- case 1:
- client->flags |= I2C_CLIENT_PEC;
- break;
- default:
- return -EINVAL;
- }
-
- return count;
-}
-
-static DEVICE_ATTR_RW(pec);
-
static int lm90_temp_get_resolution(struct lm90_data *data, int index)
{
switch (index) {
@@ -2659,11 +2623,6 @@ static irqreturn_t lm90_irq_thread(int irq, void *dev_id)
return IRQ_NONE;
}
-static void lm90_remove_pec(void *dev)
-{
- device_remove_file(dev, &dev_attr_pec);
-}
-
static int lm90_probe_channel_from_dt(struct i2c_client *client,
struct device_node *child,
struct lm90_data *data)
@@ -2802,6 +2761,8 @@ static int lm90_probe(struct i2c_client *client)
data->chip_config[0] |= HWMON_C_UPDATE_INTERVAL;
if (data->flags & LM90_HAVE_FAULTQUEUE)
data->chip_config[0] |= HWMON_C_TEMP_SAMPLES;
+ if (data->flags & (LM90_HAVE_PEC | LM90_HAVE_PARTIAL_PEC))
+ data->chip_config[0] |= HWMON_C_PEC;
data->info[1] = &data->temp_info;
info = &data->temp_info;
@@ -2878,19 +2839,6 @@ static int lm90_probe(struct i2c_client *client)
return err;
}
- /*
- * The 'pec' attribute is attached to the i2c device and thus created
- * separately.
- */
- if (data->flags & (LM90_HAVE_PEC | LM90_HAVE_PARTIAL_PEC)) {
- err = device_create_file(dev, &dev_attr_pec);
- if (err)
- return err;
- err = devm_add_action_or_reset(dev, lm90_remove_pec, dev);
- if (err)
- return err;
- }
-
hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
data, &data->chip,
NULL);
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread