Add a generic virtual thermal sensor

This adds the device tree bidings for the virtual thermal sensor.

The virtual sensor could be used to a temperature computed from many thermal sensors.

Signed-off-by Alexandre Bailon <[email protected]>

---
 .../thermal/virtual,thermal-sensor.yaml       | 67 +++++++++++++++++++
 include/dt-bindings/thermal/virtual-sensor.h  | 15 +++++
 2 files changed, 82 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/virtual,thermal-sensor.yaml
 create mode 100644 include/dt-bindings/thermal/virtual-sensor.h

Documentation/devicetree/bindings/thermal/virtual,thermal-sensor.yaml new file +5 -0

new file mode 100644
index 0000000000000..848b5912c79f1
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2021 BayLibre
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/thermal-sensor.yaml#

Documentation/devicetree/bindings/thermal/virtual,thermal-sensor.yaml new file +37 -0

+    description:
+      Used to uniquely identify a thermal sensor instance within an IC. Will be
+      0 on sensor nodes with only a single sensor and at least 1 on nodes
+      containing several internal sensors.
+    enum: [0, 1]
+
+  type:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Used to select the operations to perform on the sensors to get the virtual
+      sensor temperature.
+    enum:
+      - VIRTUAL_SENSOR_MIN
+      - VIRTUAL_SENSOR_MAX
+      - VIRTUAL_SENSOR_AVG
+
+  thermal-sensors:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      The thermal sensor phandle and sensor specifier used to monitor this
+      thermal zone.
+
+required:
+  - "#thermal-sensor-cells"
+  - type
+  - thermal-sensors
+
+additionalProperties: true
+
+examples:
+  - |
+    #include <dt-bindings/thermal/thermal.h>
+    #include <dt-bindings/thermal/virtual-sensor.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/mt8192-clk.h>
+
+    lvts: lvts@1100b000 {

Documentation/devicetree/bindings/thermal/virtual,thermal-sensor.yaml new file +5 -0

+      #thermal-sensor-cells = <1>;
+      type = <VIRTUAL_SENSOR_MAX>;
+      thermal-sensors = <&lvts 0>, <&lvts 1>;
+    };
+...

include/dt-bindings/thermal/virtual-sensor.h new file +0 -0

new file mode 100644
index 0000000000000..b3e4032f6f62b

include/dt-bindings/thermal/virtual-sensor.h new file +2 -1

+
+#endif /* _DT_BINDINGS_THERMAL_VIRTUAL_SENSOR_H */
--

This adds a virtual thermal sensor that reads temperature from hardware sensor and return an aggregated temperature.

Currently, this only return the max temperature.

Signed-off-by Alexandre Bailon <[email protected]>

---
 drivers/thermal/Kconfig          |   8 +
 drivers/thermal/Makefile         |   1 +
 drivers/thermal/virtual-sensor.h |  51 ++++
 drivers/thermal/virtual_sensor.c | 400 +++++++++++++++++++++++++++++++
 4 files changed, 460 insertions(+)
 create mode 100644 drivers/thermal/virtual-sensor.h
 create mode 100644 drivers/thermal/virtual_sensor.c

drivers/thermal/Kconfig +0 -0

index 7a4ba50ba97d0..23dc903da2fc5 100644

drivers/thermal/Kconfig +6 -0

+	depends on THERMAL_OF || COMPILE_TEST
+	help
+	  This option enables the generic thermal sensor aggregator.
+	  This driver creates a thermal sensor that reads the hardware sensors
+	  and aggregate the temperature.
+
 config HISI_THERMAL
 	tristate "Hisilicon thermal driver"
 	depends on ARCH_HISI || COMPILE_TEST

drivers/thermal/Makefile +1 -0

index 9729a2b089919..76dfa1d61bfc5 100644
@@ -60,3 +60,4 @@ obj-$(CONFIG_UNIPHIER_THERMAL)	+= uniphier_thermal.o
 obj-$(CONFIG_AMLOGIC_THERMAL)     += amlogic_thermal.o
 obj-$(CONFIG_SPRD_THERMAL)	+= sprd_thermal.o
 obj-$(CONFIG_KHADAS_MCU_FAN_THERMAL)	+= khadas_mcu_fan.o
+obj-$(CONFIG_VIRTUAL_THERMAL) += virtual_sensor.o

drivers/thermal/virtual-sensor.h new file +0 -0

new file mode 100644
index 0000000000000..e024d434856c7

drivers/thermal/virtual-sensor.h new file +41 -0

+
+#ifdef CONFIG_VIRTUAL_THERMAL
+struct virtual_sensor_data *
+thermal_virtual_sensor_register(struct device *dev, int sensor_id, void *data,
+				const struct thermal_zone_of_device_ops *ops);
+void thermal_virtual_sensor_unregister(struct device *dev,
+				       struct virtual_sensor_data *sensor_data);
+struct virtual_sensor_data *
+devm_thermal_virtual_sensor_register(struct device *dev, int sensor_id, void *data,
+				     const struct thermal_zone_of_device_ops *ops);
+
+void devm_thermal_virtual_sensor_unregister(struct device *dev,
+					    struct virtual_sensor *sensor);
+#else
+static inline struct virtual_sensor_data *
+thermal_virtual_sensor_register(struct device *dev, int sensor_id, void *data,
+				const struct thermal_zone_of_device_ops *ops)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+void thermal_virtual_sensor_unregister(struct device *dev,
+				       struct virtual_sensor_data *sensor_data)
+{
+}
+
+static inline struct virtual_sensor_data *
+devm_thermal_virtual_sensor_register(struct device *dev, int sensor_id, void *data,
+				     const struct thermal_zone_of_device_ops *ops)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline
+void devm_thermal_virtual_sensor_unregister(struct device *dev,
+					    struct virtual_sensor *sensor)
+{
+}
+#endif
+
+#endif /* __THERMAL_VIRTUAL_SENSOR_H__ */

drivers/thermal/virtual_sensor.c new file +10 -0

new file mode 100644
index 0000000000000..e5bb0ef9adb39
@@ -0,0 +1,400 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021 BayLibre
+ */
+
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>

drivers/thermal/virtual_sensor.c new file +90 -0

+	struct list_head node;
+
+	/* sensor interface */
+	int id;
+	void *sensor_data;
+	const struct thermal_zone_of_device_ops *ops;
+};
+
+struct virtual_sensor {
+	int count;
+	struct virtual_sensor_data *sensors;
+	struct thermal_zone_device *tzd;
+
+	struct list_head node;
+};
+
+static LIST_HEAD(thermal_sensors);
+static LIST_HEAD(virtual_sensors);
+
+static int virtual_sensor_get_temp_max(void *data, int *temperature)
+{
+	struct virtual_sensor *sensor = data;
+	int max_temp = INT_MIN;
+	int temp;
+	int i;
+
+	for (i = 0; i < sensor->count; i++) {
+		struct virtual_sensor_data *hw_sensor;
+
+		hw_sensor = &sensor->sensors[i];
+		if (!hw_sensor->ops)
+			return -ENODEV;
+
+		hw_sensor->ops->get_temp(hw_sensor->sensor_data, &temp);
+		max_temp = max(max_temp, temp);
+	}
+
+	*temperature = max_temp;
+
+	return 0;
+}
+
+static const struct thermal_zone_of_device_ops virtual_sensor_max_ops = {
+	.get_temp = virtual_sensor_get_temp_max,
+};
+
+static int virtual_sensor_get_temp_min(void *data, int *temperature)
+{
+	struct virtual_sensor *sensor = data;
+	int min_temp = INT_MAX;
+	int temp;
+	int i;
+
+	for (i = 0; i < sensor->count; i++) {
+		struct virtual_sensor_data *hw_sensor;
+
+		hw_sensor = &sensor->sensors[i];
+		if (!hw_sensor->ops)
+			return -ENODEV;
+
+		hw_sensor->ops->get_temp(hw_sensor->sensor_data, &temp);
+		min_temp = min(min_temp, temp);
+	}
+
+	*temperature = min_temp;
+
+	return 0;
+}
+
+static const struct thermal_zone_of_device_ops virtual_sensor_min_ops = {
+	.get_temp = virtual_sensor_get_temp_min,
+};
+
+static int do_avg(int val1, int val2)
+{
+	return ((val1) / 2) + ((val2) / 2) + (((val1) % 2 + (val2) % 2) / 2);
+}
+
+static int virtual_sensor_get_temp_avg(void *data, int *temperature)
+{
+	struct virtual_sensor *sensor = data;
+	int avg_temp = 0;
+	int temp;
+	int i;
+
+	for (i = 0; i < sensor->count; i++) {
+		struct virtual_sensor_data *hw_sensor;
+
+		hw_sensor = &sensor->sensors[i];
+		if (!hw_sensor->ops)

drivers/thermal/virtual_sensor.c new file +47 -0

+{
+	struct virtual_sensor_data *sensor_data;
+	int id;
+
+	list_for_each_entry(sensor_data, &thermal_sensors, node) {
+		id = args.args_count ? args.args[0] : 0;
+		if (sensor_data->id == id) {
+			memcpy(&sensor->sensors[index], sensor_data,
+				sizeof(*sensor_data));
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+
+static int virtual_sensor_probe(struct platform_device *pdev)
+{
+	const struct thermal_zone_of_device_ops *ops;
+	struct virtual_sensor *sensor;
+	struct device *dev = &pdev->dev;
+	struct of_phandle_args args;
+	u32 type;
+	int ret;
+	int i;
+
+	sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
+	if (!sensor)
+		return -ENOMEM;
+
+	sensor->count = of_count_phandle_with_args(dev->of_node,
+						   "thermal-sensors",
+						   "#thermal-sensor-cells");
+	if (sensor->count <= 0)
+		return -EINVAL;
+
+	sensor->sensors = devm_kmalloc_array(dev, sensor->count,
+					     sizeof(*sensor->sensors),
+					     GFP_KERNEL);
+	if (!sensor->sensors)
+		return -ENOMEM;
+
+	for (i = 0; i < sensor->count; i++) {
+		ret = of_parse_phandle_with_args(dev->of_node,
+						 "thermal-sensors",
+						 "#thermal-sensor-cells",
+						 i,

drivers/thermal/virtual_sensor.c new file +13 -0

+	if (ret)
+		return ret;
+
+	switch (type) {
+	case VIRTUAL_SENSOR_MAX:
+		ops = &virtual_sensor_max_ops;
+		break;
+	case VIRTUAL_SENSOR_MIN:
+		ops = &virtual_sensor_min_ops;
+		break;
+	case VIRTUAL_SENSOR_AVG:
+		ops = &virtual_sensor_avg_ops;
+		break;

drivers/thermal/virtual_sensor.c new file +5 -0

+
+	return 0;
+}
+
+static int virtual_sensor_remove(struct platform_device *pdev)

drivers/thermal/virtual_sensor.c new file +8 -0

+
+	return 0;
+}
+
+static const struct of_device_id virtual_sensor_of_match[] = {
+	{
+		.compatible = "virtual,thermal-sensor",
+	},

drivers/thermal/virtual_sensor.c new file +36 -0

+ * @data: a private pointer (owned by the caller) that will be passed
+ *        back, when a temperature reading is needed.
+ * @ops: struct thermal_zone_of_device_ops *. Must contain at least .get_temp.
+ *
+ * This function will register a thermal sensor to make it available for later
+ * usage by a virtual sensor.
+ *
+ * The thermal zone temperature is provided by the @get_temp function
+ * pointer. When called, it will have the private pointer @data back.
+ *
+ * Return: On success returns a valid struct thermal_zone_device,
+ * otherwise, it returns a corresponding ERR_PTR(). Caller must
+ * check the return value with help of IS_ERR() helper.
+ */
+struct virtual_sensor_data *thermal_virtual_sensor_register(
+	struct device *dev, int sensor_id, void *data,
+	const struct thermal_zone_of_device_ops *ops)
+{
+	struct virtual_sensor_data *sensor_data;
+
+	sensor_data = devm_kzalloc(dev, sizeof(*sensor_data), GFP_KERNEL);
+	if (!sensor_data)
+		return ERR_PTR(-ENOMEM);
+
+	sensor_data->id = sensor_id;
+	sensor_data->sensor_data = data;
+	sensor_data->ops = ops;
+
+	list_add(&sensor_data->node, &thermal_sensors);
+
+	return sensor_data;
+}
+EXPORT_SYMBOL_GPL(thermal_virtual_sensor_register);
+
+/**
+ * thermal_virtual_sensor_unregister - unregisters a sensor

drivers/thermal/virtual_sensor.c new file +5 -0

+	struct virtual_sensor *sensor;
+	int i;
+
+	list_del(&sensor_data->node);
+

drivers/thermal/virtual_sensor.c new file +11 -0

+}
+EXPORT_SYMBOL_GPL(thermal_virtual_sensor_unregister);
+
+static void devm_thermal_virtual_sensor_release(struct device *dev, void *res)
+{
+	thermal_virtual_sensor_unregister(dev,
+					  *(struct virtual_sensor_data **)res);
+}
+
+static int devm_thermal_virtual_sensor_match(struct device *dev, void *res,
+					     void *data)

Quoted 2 lines · Alexandre Bailon

This adds a virtual thermal sensor that reads temperature from hardware sensor and return an aggregated temperature.

s/hardware sensor/hardware sensors/

Quoted 1 line

Currently, this only return the max temperature.

it seems this is outdated.

Quoted 15 lines

Signed-off-by Alexandre Bailon <[email protected]>

---
  drivers/thermal/Kconfig          |   8 +
  drivers/thermal/Makefile         |   1 +
  drivers/thermal/virtual-sensor.h |  51 ++++
  drivers/thermal/virtual_sensor.c | 400 +++++++++++++++++++++++++++++++
  4 files changed, 460 insertions(+)

create mode 100644 drivers/thermal/virtual-sensor.h create mode 100644 drivers/thermal/virtual_sensor.c

drivers/thermal/Kconfig +0 -0

index 7a4ba50ba97d0..23dc903da2fc5 100644

drivers/thermal/Kconfig config THERMAL_MMIO · line 232

Quoted 5 lines

drivers/thermal/Kconfig +2 -0

@@ -228,6 +228,14 @@ config THERMAL_MMIO
  	  register or shared memory, is a potential candidate to work with this
  	  driver.
 
+config VIRTUAL_THERMAL
+	tristate "Generic virtual thermal sensor driver"

Not sure if 'Generic' adds much value here.

Quoted 29 lines

drivers/thermal/Kconfig +6 -0

+	depends on THERMAL_OF || COMPILE_TEST
+	help
+	  This option enables the generic thermal sensor aggregator.
+	  This driver creates a thermal sensor that reads the hardware sensors
+	  and aggregate the temperature.
+
  config HISI_THERMAL
  	tristate "Hisilicon thermal driver"
  	depends on ARCH_HISI || COMPILE_TEST

drivers/thermal/Makefile +1 -0

index 9729a2b089919..76dfa1d61bfc5 100644
@@ -60,3 +60,4 @@ obj-$(CONFIG_UNIPHIER_THERMAL)	+= uniphier_thermal.o
  obj-$(CONFIG_AMLOGIC_THERMAL)     += amlogic_thermal.o
  obj-$(CONFIG_SPRD_THERMAL)	+= sprd_thermal.o
  obj-$(CONFIG_KHADAS_MCU_FAN_THERMAL)	+= khadas_mcu_fan.o
+obj-$(CONFIG_VIRTUAL_THERMAL) += virtual_sensor.o

drivers/thermal/virtual-sensor.h new file +5 -0

new file mode 100644
index 0000000000000..e024d434856c7
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 BayLibre
+ */
+

drivers/thermal/virtual-sensor.h line 10

Quoted 5 lines

drivers/thermal/virtual-sensor.h new file +5 -0

+#ifndef __THERMAL_VIRTUAL_SENSOR_H__
+#define __THERMAL_VIRTUAL_SENSOR_H__
+
+struct virtual_sensor;
+struct virtual_sensor_data;

Other types of virtual sensors could be added in the future, you might want to name these virtual_thermal_sensor(_data). Then again, these structs are of internal use only, so it's probably not super important.

Quoted 58 lines

drivers/thermal/virtual-sensor.h new file +41 -0

+
+#ifdef CONFIG_VIRTUAL_THERMAL
+struct virtual_sensor_data *
+thermal_virtual_sensor_register(struct device *dev, int sensor_id, void *data,
+				const struct thermal_zone_of_device_ops *ops);
+void thermal_virtual_sensor_unregister(struct device *dev,
+				       struct virtual_sensor_data *sensor_data);
+struct virtual_sensor_data *
+devm_thermal_virtual_sensor_register(struct device *dev, int sensor_id, void *data,
+				     const struct thermal_zone_of_device_ops *ops);
+
+void devm_thermal_virtual_sensor_unregister(struct device *dev,
+					    struct virtual_sensor *sensor);
+#else
+static inline struct virtual_sensor_data *
+thermal_virtual_sensor_register(struct device *dev, int sensor_id, void *data,
+				const struct thermal_zone_of_device_ops *ops)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+void thermal_virtual_sensor_unregister(struct device *dev,
+				       struct virtual_sensor_data *sensor_data)
+{
+}
+
+static inline struct virtual_sensor_data *
+devm_thermal_virtual_sensor_register(struct device *dev, int sensor_id, void *data,
+				     const struct thermal_zone_of_device_ops *ops)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline
+void devm_thermal_virtual_sensor_unregister(struct device *dev,
+					    struct virtual_sensor *sensor)
+{
+}
+#endif
+
+#endif /* __THERMAL_VIRTUAL_SENSOR_H__ */

drivers/thermal/virtual_sensor.c new file +10 -0

new file mode 100644
index 0000000000000..e5bb0ef9adb39
@@ -0,0 +1,400 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021 BayLibre
+ */
+
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>

drivers/thermal/virtual_sensor.c line 20

Quoted 10 lines

drivers/thermal/virtual_sensor.c new file +10 -0

+#include <linux/slab.h>
+#include <linux/thermal.h>
+#include <linux/types.h>
+#include <linux/string.h>
+
+#include <dt-bindings/thermal/virtual-sensor.h>
+
+#include "virtual-sensor.h"
+
+struct virtual_sensor_data {

This struct (typically) corresponds to an actual sensor, maybe name it 'thermal_sensor_data' instead?

Quoted 90 lines

drivers/thermal/virtual_sensor.c new file +90 -0

+	struct list_head node;
+
+	/* sensor interface */
+	int id;
+	void *sensor_data;
+	const struct thermal_zone_of_device_ops *ops;
+};
+
+struct virtual_sensor {
+	int count;
+	struct virtual_sensor_data *sensors;
+	struct thermal_zone_device *tzd;
+
+	struct list_head node;
+};
+
+static LIST_HEAD(thermal_sensors);
+static LIST_HEAD(virtual_sensors);
+
+static int virtual_sensor_get_temp_max(void *data, int *temperature)
+{
+	struct virtual_sensor *sensor = data;
+	int max_temp = INT_MIN;
+	int temp;
+	int i;
+
+	for (i = 0; i < sensor->count; i++) {
+		struct virtual_sensor_data *hw_sensor;
+
+		hw_sensor = &sensor->sensors[i];
+		if (!hw_sensor->ops)
+			return -ENODEV;
+
+		hw_sensor->ops->get_temp(hw_sensor->sensor_data, &temp);
+		max_temp = max(max_temp, temp);
+	}
+
+	*temperature = max_temp;
+
+	return 0;
+}
+
+static const struct thermal_zone_of_device_ops virtual_sensor_max_ops = {
+	.get_temp = virtual_sensor_get_temp_max,
+};
+
+static int virtual_sensor_get_temp_min(void *data, int *temperature)
+{
+	struct virtual_sensor *sensor = data;
+	int min_temp = INT_MAX;
+	int temp;
+	int i;
+
+	for (i = 0; i < sensor->count; i++) {
+		struct virtual_sensor_data *hw_sensor;
+
+		hw_sensor = &sensor->sensors[i];
+		if (!hw_sensor->ops)
+			return -ENODEV;
+
+		hw_sensor->ops->get_temp(hw_sensor->sensor_data, &temp);
+		min_temp = min(min_temp, temp);
+	}
+
+	*temperature = min_temp;
+
+	return 0;
+}
+
+static const struct thermal_zone_of_device_ops virtual_sensor_min_ops = {
+	.get_temp = virtual_sensor_get_temp_min,
+};
+
+static int do_avg(int val1, int val2)
+{
+	return ((val1) / 2) + ((val2) / 2) + (((val1) % 2 + (val2) % 2) / 2);
+}
+
+static int virtual_sensor_get_temp_avg(void *data, int *temperature)
+{
+	struct virtual_sensor *sensor = data;
+	int avg_temp = 0;
+	int temp;
+	int i;
+
+	for (i = 0; i < sensor->count; i++) {
+		struct virtual_sensor_data *hw_sensor;
+
+		hw_sensor = &sensor->sensors[i];
+		if (!hw_sensor->ops)

drivers/thermal/virtual_sensor.c line 120

Quoted 10 lines

drivers/thermal/virtual_sensor.c new file +10 -0

+			return -ENODEV;
+
+		hw_sensor->ops->get_temp(hw_sensor->sensor_data, &temp);
+		avg_temp = do_avg(avg_temp, temp);
+	}
+
+	*temperature = avg_temp;
+
+	return 0;
+}

_get_temp_min(), _get_temp_max() and _get_temp_avg() have the same structure, the only differences is the aggregator function and the initialization value. If you wanted to save a few lines of code you could have a meta-function that receives the initialization value and a pointer of the aggregator function.

The functions are relatively short though, so I wouldn't claim that it would be a huge improvement, one could also argue that the code is easier to follow as is.

drivers/thermal/virtual_sensor.c line 128

Quoted 9 lines

drivers/thermal/virtual_sensor.c new file +8 -0

+
+static const struct thermal_zone_of_device_ops virtual_sensor_avg_ops = {
+	.get_temp = virtual_sensor_get_temp_avg,
+};
+
+static int register_virtual_sensor(struct virtual_sensor *sensor,
+				    struct of_phandle_args args,
+				    int index)

Does this really register a virtual sensor? IIUC the registered sensor is (typically) an actual sensor, which is used by a virtual sensor.

Shouldn't it be something like 'register_thermal_sensor' or 'virtual_sensor_add_sensor'?

Quoted 48 lines

drivers/thermal/virtual_sensor.c new file +47 -0

+{
+	struct virtual_sensor_data *sensor_data;
+	int id;
+
+	list_for_each_entry(sensor_data, &thermal_sensors, node) {
+		id = args.args_count ? args.args[0] : 0;
+		if (sensor_data->id == id) {
+			memcpy(&sensor->sensors[index], sensor_data,
+				sizeof(*sensor_data));
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+
+static int virtual_sensor_probe(struct platform_device *pdev)
+{
+	const struct thermal_zone_of_device_ops *ops;
+	struct virtual_sensor *sensor;
+	struct device *dev = &pdev->dev;
+	struct of_phandle_args args;
+	u32 type;
+	int ret;
+	int i;
+
+	sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
+	if (!sensor)
+		return -ENOMEM;
+
+	sensor->count = of_count_phandle_with_args(dev->of_node,
+						   "thermal-sensors",
+						   "#thermal-sensor-cells");
+	if (sensor->count <= 0)
+		return -EINVAL;
+
+	sensor->sensors = devm_kmalloc_array(dev, sensor->count,
+					     sizeof(*sensor->sensors),
+					     GFP_KERNEL);
+	if (!sensor->sensors)
+		return -ENOMEM;
+
+	for (i = 0; i < sensor->count; i++) {
+		ret = of_parse_phandle_with_args(dev->of_node,
+						 "thermal-sensors",
+						 "#thermal-sensor-cells",
+						 i,

drivers/thermal/virtual_sensor.c line 185

Quoted 10 lines

drivers/thermal/virtual_sensor.c new file +10 -0

+						 &args);
+		if (ret)
+			return ret;
+
+		ret = register_virtual_sensor(sensor, args, i);
+		if (ret)
+			return ret;
+	}
+
+	ret = of_property_read_u32(dev->of_node, "type", &type);

More a question for the binding, butthis should probably be something more specific than 'type', like 'aggregation-function'.

Quoted 18 lines

drivers/thermal/virtual_sensor.c new file +18 -0

+	if (ret)
+		return ret;
+
+	switch (type) {
+	case VIRTUAL_SENSOR_MAX:
+		ops = &virtual_sensor_max_ops;
+		break;
+	case VIRTUAL_SENSOR_MIN:
+		ops = &virtual_sensor_min_ops;
+		break;
+	case VIRTUAL_SENSOR_AVG:
+		ops = &virtual_sensor_avg_ops;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	sensor->tzd = devm_thermal_zone_of_sensor_register(dev, 0, sensor, ops);

drivers/thermal/virtual_sensor.c line 208

Quoted 5 lines

drivers/thermal/virtual_sensor.c new file +5 -0

+	if (IS_ERR(sensor->tzd))
+		return PTR_ERR(sensor->tzd);
+
+	platform_set_drvdata(pdev, sensor);
+	list_add(&sensor->node, &virtual_sensors);

If you also added the sensor to 'thermal_sensors' you could support virtual sensors using virtual sensors, though it's not clear how useful that would be in practice and it could raise issues with the initialization order.

Quoted 10 lines

drivers/thermal/virtual_sensor.c new file +9 -0

+
+	return 0;
+}
+
+static int virtual_sensor_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct virtual_sensor *sensor;
+

drivers/thermal/virtual_sensor.c line 223

Quoted 6 lines

drivers/thermal/virtual_sensor.c new file +6 -0

+	sensor = platform_get_drvdata(pdev);
+	list_del(&sensor->node);
+
+	devm_thermal_zone_of_sensor_unregister(dev, sensor->tzd);
+	devm_kfree(dev, sensor->sensors);
+	devm_kfree(dev, sensor);

Are the above 3 statements really needed, shouldn't devm_* handle that automagically?

Quoted 9 lines

drivers/thermal/virtual_sensor.c new file +8 -0

+
+	return 0;
+}
+
+static const struct of_device_id virtual_sensor_of_match[] = {
+	{
+		.compatible = "virtual,thermal-sensor",
+	},

drivers/thermal/virtual_sensor.c line 241

Quoted 10 lines

drivers/thermal/virtual_sensor.c new file +10 -0

+	{
+	},
+};
+MODULE_DEVICE_TABLE(of, thermal_aggr_of_match);
+
+static struct platform_driver virtual_sensor = {
+	.probe = virtual_sensor_probe,
+	.remove = virtual_sensor_remove,
+	.driver = {
+		.name = "virtual-sensor",

I suggest to change it to 'virtual-thermal-sensor' since there might be other types of virtual sensors.

drivers/thermal/virtual_sensor.c line 247

Quoted 6 lines

drivers/thermal/virtual_sensor.c new file +6 -0

+		.of_match_table = virtual_sensor_of_match,
+	},
+};
+
+/**
+ * thermal_virtual_sensor_register - registers a sensor that could by a virtual

s/by/be/

drivers/thermal/virtual_sensor.c line 252

Quoted 5 lines

drivers/thermal/virtual_sensor.c new file +5 -0

+ * sensor
+ * @dev: a valid struct device pointer of a sensor device. Must contain
+ *       a valid .of_node, for the sensor node.
+ * @sensor_id: a sensor identifier, in case the sensor IP has more
+ *             than one sensors

s/sensors/sensor/

Quoted 36 lines

drivers/thermal/virtual_sensor.c new file +36 -0

+ * @data: a private pointer (owned by the caller) that will be passed
+ *        back, when a temperature reading is needed.
+ * @ops: struct thermal_zone_of_device_ops *. Must contain at least .get_temp.
+ *
+ * This function will register a thermal sensor to make it available for later
+ * usage by a virtual sensor.
+ *
+ * The thermal zone temperature is provided by the @get_temp function
+ * pointer. When called, it will have the private pointer @data back.
+ *
+ * Return: On success returns a valid struct thermal_zone_device,
+ * otherwise, it returns a corresponding ERR_PTR(). Caller must
+ * check the return value with help of IS_ERR() helper.
+ */
+struct virtual_sensor_data *thermal_virtual_sensor_register(
+	struct device *dev, int sensor_id, void *data,
+	const struct thermal_zone_of_device_ops *ops)
+{
+	struct virtual_sensor_data *sensor_data;
+
+	sensor_data = devm_kzalloc(dev, sizeof(*sensor_data), GFP_KERNEL);
+	if (!sensor_data)
+		return ERR_PTR(-ENOMEM);
+
+	sensor_data->id = sensor_id;
+	sensor_data->sensor_data = data;
+	sensor_data->ops = ops;
+
+	list_add(&sensor_data->node, &thermal_sensors);
+
+	return sensor_data;
+}
+EXPORT_SYMBOL_GPL(thermal_virtual_sensor_register);
+
+/**
+ * thermal_virtual_sensor_unregister - unregisters a sensor

drivers/thermal/virtual_sensor.c line 298

Quoted 10 lines

drivers/thermal/virtual_sensor.c new file +10 -0

+ * @dev: a valid struct device pointer of a sensor device.
+ * @sensor_data: a pointer to struct virtual_sensor_data to unregister.
+ *
+ * This function removes the sensor from the list of available thermal sensors.
+ * If the sensor is in use, then the next call to .get_temp will return -ENODEV.
+ */
+void thermal_virtual_sensor_unregister(struct device *dev,
+				       struct virtual_sensor_data *sensor_data)
+{
+	struct virtual_sensor_data *temp;

'temp' might not be the best name in this context, since it's associated with temperature. Maybe name it 'sd'?

Quoted 9 lines

drivers/thermal/virtual_sensor.c new file +9 -0

+	struct virtual_sensor *sensor;
+	int i;
+
+	list_del(&sensor_data->node);
+
+	list_for_each_entry(sensor, &virtual_sensors, node) {
+		for (i = 0; i < sensor->count; i++) {
+			temp = &sensor->sensors[i];
+			if (temp->id == sensor_data->id &&

drivers/thermal/virtual_sensor.c line 313

Quoted 6 lines

drivers/thermal/virtual_sensor.c new file +6 -0

+				temp->sensor_data == sensor_data->sensor_data) {
+				temp->ops = NULL;
+			}
+		}
+	}
+	devm_kfree(dev, sensor_data);

Does it actually make sense to allocate the memory with devm_kzalloc() if it is explicitly freed here?

Quoted 12 lines

drivers/thermal/virtual_sensor.c new file +11 -0

+}
+EXPORT_SYMBOL_GPL(thermal_virtual_sensor_unregister);
+
+static void devm_thermal_virtual_sensor_release(struct device *dev, void *res)
+{
+	thermal_virtual_sensor_unregister(dev,
+					  *(struct virtual_sensor_data **)res);
+}
+
+static int devm_thermal_virtual_sensor_match(struct device *dev, void *res,
+					     void *data)

drivers/thermal/virtual_sensor.c line 334

Quoted 10 lines

drivers/thermal/virtual_sensor.c new file +10 -0

+{
+	struct virtual_sensor_data **r = res;
+
+	if (WARN_ON(!r || !*r))
+		return 0;
+
+	return *r == data;
+}
+
+

delete one of the empty lines

drivers/thermal/virtual_sensor.c line 341

Quoted 7 lines

drivers/thermal/virtual_sensor.c new file +7 -0

+/**
+ * devm_thermal_virtual_sensor_register - Resource managed version of
+ *				thermal_virtual_sensor_register()
+ * @dev: a valid struct device pointer of a sensor device. Must contain
+ *       a valid .of_node, for the sensor node.
+ * @sensor_id: a sensor identifier, in case the sensor IP has more
+ *	       than one sensors

s/sensors/sensor/

drivers/thermal/virtual_sensor.c line 352

Quoted 11 lines

drivers/thermal/virtual_sensor.c new file +11 -0

+ * @data: a private pointer (owned by the caller) that will be passed
+ *	  back, when a temperature reading is needed.
+ * @ops: struct thermal_zone_of_device_ops *. Must contain at least .get_temp.
+ *
+ * Refer thermal_zone_of_sensor_register() for more details.
+ *
+ * Return: On success returns a valid struct virtual_sensor_data,
+ * otherwise, it returns a corresponding ERR_PTR(). Caller must
+ * check the return value with help of IS_ERR() helper.
+ * Registered virtual_sensor_data device will automatically be
+ * released when device is unbounded.

s/unbounded/unbound/