thermal: hwmon: Use extra_groups for adding temperature attributes

Instead of passing NULL as the last argument to __hwmon_device_register()
in hwmon_device_register_for_thermal() and then adding each temperature
sysfs attribute to the hwmon device via device_create_file(), redefine
hwmon_device_register_for_thermal() to take an extra_groups argument
that will be passed to __hwmon_device_register(), define an attribute
group with a proper .is_visible() callback for the temperature
attributes and a related attribute groups pointer, and pass the latter
to hwmon_device_register_for_thermal().

This causes the code to be way more straightforward and closer to
what the other users of the hwmon subsystem do.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Link: https://patch.msgid.link/8704209.T7Z3S40VBb@rafael.j.wysocki
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 6812d1fd7c283a3282d16b5169b7a6aadbf3b4bc..c16e6da0690ce192379b1d897f380424ad352e85 100644 (file)
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -1082,6 +1082,7 @@ EXPORT_SYMBOL_GPL(hwmon_device_register_with_info);
  * @dev: the parent device
  * @name: hwmon name attribute
  * @drvdata: driver data to attach to created device
+ * @extra_groups: pointer to list of additional non-standard attribute groups
  *
  * The use of this function is restricted. It is provided for legacy reasons
  * and must only be called from the thermal subsystem.
@@ -1093,12 +1094,13 @@ EXPORT_SYMBOL_GPL(hwmon_device_register_with_info);
  */
 struct device *
 hwmon_device_register_for_thermal(struct device *dev, const char *name,
-                                 void *drvdata)
+                                 void *drvdata,
+                                 const struct attribute_group **extra_groups)
 {
        if (!name || !dev)
                return ERR_PTR(-EINVAL);
 
-       return __hwmon_device_register(dev, name, drvdata, NULL, NULL);
+       return __hwmon_device_register(dev, name, drvdata, NULL, extra_groups);
 }
 EXPORT_SYMBOL_NS_GPL(hwmon_device_register_for_thermal, "HWMON_THERMAL");
 

diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
index 223ae1571655bd9ace2a224b1be5f2827ae31a66..386dfb9f559e33fbbdf722aacba8bb8cea1a8112 100644 (file)
--- a/drivers/thermal/thermal_hwmon.c
+++ b/drivers/thermal/thermal_hwmon.c
@@ -25,25 +25,13 @@
  */
 #define THERMAL_HWMON_NAME_LENGTH (THERMAL_NAME_LENGTH + 11)
 
-struct thermal_hwmon_attr {
-       struct device_attribute attr;
-};
-
-/* one temperature input for each thermal zone */
-struct thermal_hwmon_temp {
-       struct thermal_zone_device *tz;
-       struct thermal_hwmon_attr temp_input;   /* hwmon sys attr */
-       struct thermal_hwmon_attr temp_crit;    /* hwmon sys attr */
-       bool temp_crit_present;
-};
-
 /* hwmon sys I/F */
 /* thermal zone devices with the same type share one hwmon device */
 struct thermal_hwmon_device {
        char name[THERMAL_HWMON_NAME_LENGTH];
        struct device *device;
        struct list_head node;
-       struct thermal_hwmon_temp tz_temp;
+       struct thermal_zone_device *tz;
 };
 
 static LIST_HEAD(thermal_hwmon_list);
@@ -51,19 +39,14 @@ static LIST_HEAD(thermal_hwmon_list);
 static DEFINE_MUTEX(thermal_hwmon_list_lock);
 
 static ssize_t
-temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
+temp1_input_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
+       struct thermal_hwmon_device *hwmon = dev_get_drvdata(dev);
+       struct thermal_zone_device *tz = hwmon->tz;
        int temperature;
        int ret;
-       struct thermal_hwmon_attr *hwmon_attr
-                       = container_of(attr, struct thermal_hwmon_attr, attr);
-       struct thermal_hwmon_temp *temp
-                       = container_of(hwmon_attr, struct thermal_hwmon_temp,
-                                      temp_input);
-       struct thermal_zone_device *tz = temp->tz;
 
        ret = thermal_zone_get_temp(tz, &temperature);
-
        if (ret)
                return ret;
 
@@ -71,14 +54,10 @@ temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
 }
 
 static ssize_t
-temp_crit_show(struct device *dev, struct device_attribute *attr, char *buf)
+temp1_crit_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
-       struct thermal_hwmon_attr *hwmon_attr
-                       = container_of(attr, struct thermal_hwmon_attr, attr);
-       struct thermal_hwmon_temp *temp
-                       = container_of(hwmon_attr, struct thermal_hwmon_temp,
-                                      temp_crit);
-       struct thermal_zone_device *tz = temp->tz;
+       struct thermal_hwmon_device *hwmon = dev_get_drvdata(dev);
+       struct thermal_zone_device *tz = hwmon->tz;
        int temperature;
        int ret;
 
@@ -91,22 +70,49 @@ temp_crit_show(struct device *dev, struct device_attribute *attr, char *buf)
        return sysfs_emit(buf, "%d\n", temperature);
 }
 
-static bool thermal_zone_crit_temp_valid(struct thermal_zone_device *tz)
+static DEVICE_ATTR_RO(temp1_input);
+static DEVICE_ATTR_RO(temp1_crit);
+
+static struct attribute *thermal_hwmon_attrs[] = {
+       &dev_attr_temp1_input.attr,
+       &dev_attr_temp1_crit.attr,
+       NULL,
+};
+
+static umode_t thermal_hwmon_attr_is_visible(struct kobject *kobj,
+                                            struct attribute *a, int n)
 {
-       int temp;
-       return tz->ops.get_crit_temp && !tz->ops.get_crit_temp(tz, &temp);
+       if (a == &dev_attr_temp1_input.attr)
+               return a->mode;
+
+       if (a == &dev_attr_temp1_crit.attr) {
+               struct thermal_hwmon_device *hwmon = dev_get_drvdata(kobj_to_dev(kobj));
+               struct thermal_zone_device *tz = hwmon->tz;
+               int dummy;
+
+               if (tz->ops.get_crit_temp && !tz->ops.get_crit_temp(tz, &dummy))
+                       return a->mode;
+       }
+
+       return 0;
 }
 
+static const struct attribute_group thermal_hwmon_group = {
+       .attrs  = thermal_hwmon_attrs,
+       .is_visible = thermal_hwmon_attr_is_visible,
+};
+
+__ATTRIBUTE_GROUPS(thermal_hwmon);
+
 int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
 {
        struct thermal_hwmon_device *hwmon;
-       struct thermal_hwmon_temp *temp;
-       int result;
 
        hwmon = kzalloc_obj(*hwmon);
        if (!hwmon)
                return -ENOMEM;
 
+       hwmon->tz = tz;
        /*
         * Append the thermal zone ID preceded by an underline character to the
         * type to disambiguate the sensors command output.
@@ -114,35 +120,13 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
        scnprintf(hwmon->name, THERMAL_HWMON_NAME_LENGTH, "%s_%d", tz->type, tz->id);
        strreplace(hwmon->name, '-', '_');
        hwmon->device = hwmon_device_register_for_thermal(&tz->device,
-                                                         hwmon->name, hwmon);
+                                                         hwmon->name, hwmon,
+                                                         thermal_hwmon_groups);
        if (IS_ERR(hwmon->device)) {
-               result = PTR_ERR(hwmon->device);
-               goto free_mem;
-       }
+               int result = PTR_ERR(hwmon->device);
 
-       temp = &hwmon->tz_temp;
-
-       temp->tz = tz;
-
-       temp->temp_input.attr.attr.name = "temp1_input";
-       temp->temp_input.attr.attr.mode = 0444;
-       temp->temp_input.attr.show = temp_input_show;
-       sysfs_attr_init(&temp->temp_input.attr.attr);
-       result = device_create_file(hwmon->device, &temp->temp_input.attr);
-       if (result)
-               goto unregister_name;
-
-       if (thermal_zone_crit_temp_valid(tz)) {
-               temp->temp_crit.attr.attr.name = "temp1_crit";
-               temp->temp_crit.attr.attr.mode = 0444;
-               temp->temp_crit.attr.show = temp_crit_show;
-               sysfs_attr_init(&temp->temp_crit.attr.attr);
-               result = device_create_file(hwmon->device,
-                                           &temp->temp_crit.attr);
-               if (result)
-                       goto unregister_input;
-
-               temp->temp_crit_present = true;
+               kfree(hwmon);
+               return result;
        }
 
        /* The list is needed for hwmon lookup during removal. */
@@ -151,15 +135,6 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
        mutex_unlock(&thermal_hwmon_list_lock);
 
        return 0;
-
- unregister_input:
-       device_remove_file(hwmon->device, &temp->temp_input.attr);
- unregister_name:
-       hwmon_device_unregister(hwmon->device);
- free_mem:
-       kfree(hwmon);
-
-       return result;
 }
 EXPORT_SYMBOL_GPL(thermal_add_hwmon_sysfs);
 
@@ -169,7 +144,7 @@ thermal_hwmon_lookup(const struct thermal_zone_device *tz)
        struct thermal_hwmon_device *hwmon;
 
        list_for_each_entry(hwmon, &thermal_hwmon_list, node) {
-               if (hwmon->tz_temp.tz == tz)
+               if (hwmon->tz == tz)
                        return hwmon;
        }
        return NULL;
@@ -178,7 +153,6 @@ thermal_hwmon_lookup(const struct thermal_zone_device *tz)
 void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
 {
        struct thermal_hwmon_device *hwmon;
-       struct thermal_hwmon_temp *temp;
 
        scoped_guard(mutex, &thermal_hwmon_list_lock) {
                hwmon = thermal_hwmon_lookup(tz);
@@ -188,12 +162,6 @@ void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
                list_del(&hwmon->node);
        }
 
-       temp = &hwmon->tz_temp;
-
-       device_remove_file(hwmon->device, &temp->temp_input.attr);
-       if (temp->temp_crit_present)
-               device_remove_file(hwmon->device, &temp->temp_crit.attr);
-
        hwmon_device_unregister(hwmon->device);
        kfree(hwmon);
 }

diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
index 301a83afbd6636101f53fc49b151eb534426f212..a578a10baff29177c299f5b1fcb0169b7298399b 100644 (file)
--- a/include/linux/hwmon.h
+++ b/include/linux/hwmon.h
@@ -477,7 +477,8 @@ hwmon_device_register_with_info(struct device *dev,
                                const struct attribute_group **extra_groups);
 struct device *
 hwmon_device_register_for_thermal(struct device *dev, const char *name,
-                                 void *drvdata);
+                                 void *drvdata,
+                                 const struct attribute_group **extra_groups);
 struct device *
 devm_hwmon_device_register_with_info(struct device *dev,
                                const char *name, void *drvdata,