Hi Guenter,
Comments noted, thanks. I'll send a V2 later today.
Cheers,
-Paul
Le mar., déc. 21 2021 at 10:17:03 -0800, Guenter Roeck
<linux@xxxxxxxxxxxx> a écrit :
On 12/21/21 9:50 AM, Paul Cercueil wrote:
If a label is defined in the device tree for this device add that
to the device specific attributes. This is useful for userspace to
be able to identify an individual device when multiple identical
chips are present in the system.
This is an ABI change which needs to be documented in
Documentation/hwmon/sysfs-interface.rst.
Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
Tested-by: Cosmin Tanislav <cosmin.tanislav@xxxxxxxxxx>
---
drivers/hwmon/hwmon.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 3501a3ead4ba..15826260a463 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -71,8 +71,23 @@ name_show(struct device *dev, struct
device_attribute *attr, char *buf)
}
static DEVICE_ATTR_RO(name);
+static ssize_t
+label_show(struct device *dev, struct device_attribute *attr, char
*buf)
+{
+ const char *label;
+ int ret;
+
+ ret = device_property_read_string(dev, "label", &label);
Requires "#include <linux/property.h>". Also, reading and verifying
the label
each time it is read is excessive. More on that see below.
+ if (ret < 0)
+ return ret;
+
+ return sysfs_emit(buf, "%s\n", label);
+}
+static DEVICE_ATTR_RO(label);
+
static struct attribute *hwmon_dev_attrs[] = {
&dev_attr_name.attr,
+ &dev_attr_label.attr,
NULL
};
@@ -81,7 +96,12 @@ static umode_t
hwmon_dev_name_is_visible(struct kobject *kobj,
That function name is no longer appropriate and should be changed to
something like hwmon_dev_attr_is_visible.
{
struct device *dev = kobj_to_dev(kobj);
- if (to_hwmon_device(dev)->name == NULL)
+ if (attr == &dev_attr_name.attr &&
+ to_hwmon_device(dev)->name == NULL)
Unnecessary continuation line.
+ return 0;
+
+ if (attr == &dev_attr_label.attr &&
+ !device_property_present(dev, "label"))
If the property is present but not a string, each read of "label"
would
return a runtime error. I don't like that. I would suggest to store
a pointer to the label in struct hwmon_device, set it during
registration
(eg by using devm_strdup() if it is defined), and use
if (attr == &dev_attr_label.attr && to_hwmon_device(dev)->label ==
NULL)
return 0;
to check if it is present.
return 0;
return attr->mode;
