This series add a virtual thermal sensor.
It could be used to get a temperature using some thermal sensors.
Currently, the supported operations are max, min and avg.
The virtual sensor could be easily extended to support others operations.
Note:
Currently, thermal drivers must explicitly register their sensors to make them
available to the virtual sensor.
This doesn't seem a good solution to me and I think it would be preferable to
update the framework to register the list of each available sensors.
Changes in v2:
- Fix some warnings / errors reported by kernel test robot
- rename some struct and functions with a more accurate name
- update the dt bindings: rename type attribute to aggregation-function
- factorize a little bit the aggregation functions
Alexandre Bailon (2):
dt-bindings: Add bindings for the virtual thermal sensor
thermal: add a virtual sensor to aggregate temperatures
.../thermal/virtual,thermal-sensor.yaml | 67 +++
drivers/thermal/Kconfig | 8 +
drivers/thermal/Makefile | 1 +
drivers/thermal/virtual-sensor.h | 51 +++
drivers/thermal/virtual_sensor.c | 400 ++++++++++++++++++
include/dt-bindings/thermal/virtual-sensor.h | 15 +
6 files changed, 542 insertions(+)
create mode 100644 Documentation/devicetree/bindings/thermal/virtual,thermal-sensor.yaml
create mode 100644 drivers/thermal/virtual-sensor.h
create mode 100644 drivers/thermal/virtual_sensor.c
create mode 100644 include/dt-bindings/thermal/virtual-sensor.h
--
2.31.1
This adds a virtual thermal sensor that reads temperature from
hardware sensor and return an aggregated temperature.
Currently, this supports three operations:
the minimum, maximum and average temperature.
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Alexandre Bailon <[email protected]>
---
drivers/thermal/Kconfig | 8 +
drivers/thermal/Makefile | 1 +
drivers/thermal/virtual-thermal-sensor.h | 54 ++++
drivers/thermal/virtual_thermal_sensor.c | 350 +++++++++++++++++++++++
4 files changed, 413 insertions(+)
create mode 100644 drivers/thermal/virtual-thermal-sensor.h
create mode 100644 drivers/thermal/virtual_thermal_sensor.c
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index d7f44deab5b1d..20bc93c48f5b1 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -228,6 +228,14 @@ config THERMAL_MMIO
register or shared memory, is a potential candidate to work with this
driver.
+config VIRTUAL_THERMAL
+ tristate "Virtual thermal sensor driver"
+ 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
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 82fc3e616e54b..8bf55973059c5 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -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_thermal_sensor.o
diff --git a/drivers/thermal/virtual-thermal-sensor.h b/drivers/thermal/virtual-thermal-sensor.h
new file mode 100644
index 0000000000000..3bbf7c324dddc
--- /dev/null
+++ b/drivers/thermal/virtual-thermal-sensor.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 BayLibre
+ */
+
+#ifndef __THERMAL_VIRTUAL_SENSOR_H__
+#define __THERMAL_VIRTUAL_SENSOR_H__
+
+#include <linux/device.h>
+#include <linux/thermal.h>
+
+struct virtual_thermal_sensor;
+struct thermal_sensor_data;
+
+#if IS_ENABLED(CONFIG_VIRTUAL_THERMAL)
+struct thermal_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 thermal_sensor_data *sensor_data);
+struct thermal_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_thermal_sensor *sensor);
+#else
+static inline struct thermal_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 thermal_sensor_data *sensor_data)
+{
+}
+
+static inline struct thermal_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_thermal_sensor *sensor)
+{
+}
+#endif
+
+#endif /* __THERMAL_VIRTUAL_SENSOR_H__ */
diff --git a/drivers/thermal/virtual_thermal_sensor.c b/drivers/thermal/virtual_thermal_sensor.c
new file mode 100644
index 0000000000000..234563af6643e
--- /dev/null
+++ b/drivers/thermal/virtual_thermal_sensor.c
@@ -0,0 +1,350 @@
+// 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>
+#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-thermal-sensor.h"
+
+struct thermal_sensor_data {
+ struct list_head node;
+
+ /* sensor interface */
+ int id;
+ void *sensor_data;
+ const struct thermal_zone_of_device_ops *ops;
+};
+
+struct virtual_thermal_sensor {
+ int count;
+ struct thermal_sensor_data *sensors;
+ struct thermal_zone_device *tzd;
+ int (*aggr_temp)(int temp1, int temp2);
+
+ struct list_head node;
+};
+
+static LIST_HEAD(thermal_sensors);
+static LIST_HEAD(virtual_sensors);
+
+static int max_temp(int temp1, int temp2)
+{
+ return max(temp1, temp2);
+}
+
+static int min_temp(int temp1, int temp2)
+{
+ return min(temp1, temp2);
+}
+
+static int avg_temp(int temp1, int temp2)
+{
+ return ((temp1) / 2) + ((temp2) / 2) + (((temp1) % 2 + (temp2) % 2) / 2);
+}
+
+static int virtual_thermal_sensor_get_temp(void *data, int *temperature)
+{
+ struct virtual_thermal_sensor *sensor = data;
+ int max_temp = INT_MIN;
+ int temp;
+ int i;
+
+ for (i = 0; i < sensor->count; i++) {
+ struct thermal_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 = sensor->aggr_temp(max_temp, temp);
+ }
+
+ *temperature = max_temp;
+
+ return 0;
+}
+
+static const struct thermal_zone_of_device_ops virtual_thermal_sensor_ops = {
+ .get_temp = virtual_thermal_sensor_get_temp,
+};
+
+static int virtual_sensor_add_sensor(struct virtual_thermal_sensor *sensor,
+ struct of_phandle_args args,
+ int index)
+{
+ struct thermal_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_thermal_sensor_probe(struct platform_device *pdev)
+{
+ struct virtual_thermal_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,
+ &args);
+ if (ret)
+ return ret;
+
+ ret = virtual_sensor_add_sensor(sensor, args, i);
+ if (ret)
+ return ret;
+ }
+
+ ret = of_property_read_u32(dev->of_node, "aggregation-function", &type);
+ if (ret)
+ return ret;
+
+ switch (type) {
+ case VIRTUAL_THERMAL_SENSOR_MAX:
+ sensor->aggr_temp = max_temp;
+ break;
+ case VIRTUAL_THERMAL_SENSOR_MIN:
+ sensor->aggr_temp = min_temp;
+ break;
+ case VIRTUAL_THERMAL_SENSOR_AVG:
+ sensor->aggr_temp = avg_temp;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ sensor->tzd = devm_thermal_zone_of_sensor_register(dev, 0, sensor,
+ &virtual_thermal_sensor_ops);
+ if (IS_ERR(sensor->tzd))
+ return PTR_ERR(sensor->tzd);
+
+ platform_set_drvdata(pdev, sensor);
+ list_add(&sensor->node, &virtual_sensors);
+
+ return 0;
+}
+
+static int virtual_thermal_sensor_remove(struct platform_device *pdev)
+{
+ struct virtual_thermal_sensor *sensor;
+
+ sensor = platform_get_drvdata(pdev);
+ list_del(&sensor->node);
+
+ return 0;
+}
+
+static const struct of_device_id virtual_thermal_sensor_of_match[] = {
+ {
+ .compatible = "virtual,thermal-sensor",
+ },
+ {
+ },
+};
+MODULE_DEVICE_TABLE(of, virtual_thermal_sensor_of_match);
+
+static struct platform_driver virtual_thermal_sensor = {
+ .probe = virtual_thermal_sensor_probe,
+ .remove = virtual_thermal_sensor_remove,
+ .driver = {
+ .name = "virtual-thermal-sensor",
+ .of_match_table = virtual_thermal_sensor_of_match,
+ },
+};
+
+/**
+ * thermal_virtual_sensor_register - registers a sensor that could be a virtual
+ * 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 sensor
+ * @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 thermal_sensor_data *thermal_virtual_sensor_register(
+ struct device *dev, int sensor_id, void *data,
+ const struct thermal_zone_of_device_ops *ops)
+{
+ struct thermal_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
+ * @dev: a valid struct device pointer of a sensor device.
+ * @sensor_data: a pointer to struct thermal_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 thermal_sensor_data *sensor_data)
+{
+ struct thermal_sensor_data *sd;
+ struct virtual_thermal_sensor *sensor;
+ int i;
+
+ list_del(&sensor_data->node);
+
+ list_for_each_entry(sensor, &virtual_sensors, node) {
+ for (i = 0; i < sensor->count; i++) {
+ sd = &sensor->sensors[i];
+ if (sd->id == sensor_data->id &&
+ sd->sensor_data == sensor_data->sensor_data) {
+ sd->ops = NULL;
+ }
+ }
+ }
+}
+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 thermal_sensor_data **)res);
+}
+
+static int devm_thermal_virtual_sensor_match(struct device *dev, void *res,
+ void *data)
+{
+ struct thermal_sensor_data **r = res;
+
+ if (WARN_ON(!r || !*r))
+ return 0;
+
+ return *r == data;
+}
+
+/**
+ * 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 sensor
+ * @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 unbound.
+ */
+struct thermal_sensor_data *devm_thermal_virtual_sensor_register(
+ struct device *dev, int sensor_id,
+ void *data, const struct thermal_zone_of_device_ops *ops)
+{
+ struct thermal_sensor_data **ptr, *sensor_data;
+
+ ptr = devres_alloc(devm_thermal_virtual_sensor_release, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ sensor_data = thermal_virtual_sensor_register(dev, sensor_id, data, ops);
+ if (IS_ERR(sensor_data)) {
+ devres_free(ptr);
+ return sensor_data;
+ }
+
+ *ptr = sensor_data;
+ devres_add(dev, ptr);
+
+ return sensor_data;
+}
+EXPORT_SYMBOL_GPL(devm_thermal_virtual_sensor_register);
+
+/**
+ * devm_thermal_virtual_sensor_unregister - Resource managed version of
+ * thermal_virtual_sensor_unregister().
+ * @dev: Device for which resource was allocated.
+ * @sensor: a pointer to struct thermal_zone_device where the sensor is registered.
+ *
+ * This function removes the sensor from the list of sensors registered with
+ * devm_thermal_virtual_sensor_register() API.
+ * Normally this function will not need to be called and the resource
+ * management code will ensure that the resource is freed.
+ */
+void devm_thermal_virtual_sensor_unregister(struct device *dev,
+ struct virtual_thermal_sensor *sensor)
+{
+ WARN_ON(devres_release(dev, devm_thermal_virtual_sensor_release,
+ devm_thermal_virtual_sensor_match, sensor));
+}
+EXPORT_SYMBOL_GPL(devm_thermal_virtual_sensor_unregister);
+
+module_platform_driver(virtual_thermal_sensor);
+MODULE_AUTHOR("Alexandre Bailon <[email protected]>");
+MODULE_DESCRIPTION("Virtual thermal sensor");
+MODULE_LICENSE("GPL v2");
--
2.31.1
On 17/09/2021 09:27, Alexandre Bailon wrote:
> This series add a virtual thermal sensor.
> It could be used to get a temperature using some thermal sensors.
> Currently, the supported operations are max, min and avg.
> The virtual sensor could be easily extended to support others operations.
>
> Note:
> Currently, thermal drivers must explicitly register their sensors to make them
> available to the virtual sensor.
> This doesn't seem a good solution to me and I think it would be preferable to
> update the framework to register the list of each available sensors.
Why must the drivers do that ?
> Changes in v2:
> - Fix some warnings / errors reported by kernel test robot
> - rename some struct and functions with a more accurate name
> - update the dt bindings: rename type attribute to aggregation-function
> - factorize a little bit the aggregation functions
>
> Alexandre Bailon (2):
> dt-bindings: Add bindings for the virtual thermal sensor
> thermal: add a virtual sensor to aggregate temperatures
>
> .../thermal/virtual,thermal-sensor.yaml | 67 +++
> drivers/thermal/Kconfig | 8 +
> drivers/thermal/Makefile | 1 +
> drivers/thermal/virtual-sensor.h | 51 +++
> drivers/thermal/virtual_sensor.c | 400 ++++++++++++++++++
> include/dt-bindings/thermal/virtual-sensor.h | 15 +
> 6 files changed, 542 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/virtual,thermal-sensor.yaml
> create mode 100644 drivers/thermal/virtual-sensor.h
> create mode 100644 drivers/thermal/virtual_sensor.c
> create mode 100644 include/dt-bindings/thermal/virtual-sensor.h
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On Fri, Sep 17, 2021 at 9:25 AM Alexandre Bailon <[email protected]> wrote:
>
> This adds a virtual thermal sensor that reads temperature from
This should be "virtual sensor driver" I suppose.
> hardware sensor and return an aggregated temperature.
"returns"
Also wrapping it around a single hardware sensor makes a little sense
AFAICS, so I guess you mean "multiple hardware sensors" here.
> Currently, this supports three operations:
> the minimum, maximum and average temperature.
They are aggregation functions rather than "operations"
>
> Reported-by: kernel test robot <[email protected]>
This is new code, so what does it fix?
> Signed-off-by: Alexandre Bailon <[email protected]>
> ---
> drivers/thermal/Kconfig | 8 +
> drivers/thermal/Makefile | 1 +
> drivers/thermal/virtual-thermal-sensor.h | 54 ++++
> drivers/thermal/virtual_thermal_sensor.c | 350 +++++++++++++++++++++++
> 4 files changed, 413 insertions(+)
> create mode 100644 drivers/thermal/virtual-thermal-sensor.h
> create mode 100644 drivers/thermal/virtual_thermal_sensor.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index d7f44deab5b1d..20bc93c48f5b1 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -228,6 +228,14 @@ config THERMAL_MMIO
> register or shared memory, is a potential candidate to work with this
> driver.
>
> +config VIRTUAL_THERMAL
> + tristate "Virtual thermal sensor driver"
This should be "DT-based virtual thermal sensor driver" I think.
> + depends on THERMAL_OF || COMPILE_TEST
> + help
> + This option enables the generic thermal sensor aggregator.
"generic DT-based"
> + This driver creates a thermal sensor that reads the hardware sensors
s/the/multiple/
> + and aggregate the temperature.
"aggregates their output"
> +
> config HISI_THERMAL
> tristate "Hisilicon thermal driver"
> depends on ARCH_HISI || COMPILE_TEST
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 82fc3e616e54b..8bf55973059c5 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -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_thermal_sensor.o
> diff --git a/drivers/thermal/virtual-thermal-sensor.h b/drivers/thermal/virtual-thermal-sensor.h
> new file mode 100644
> index 0000000000000..3bbf7c324dddc
> --- /dev/null
> +++ b/drivers/thermal/virtual-thermal-sensor.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2021 BayLibre
> + */
> +
> +#ifndef __THERMAL_VIRTUAL_SENSOR_H__
> +#define __THERMAL_VIRTUAL_SENSOR_H__
> +
> +#include <linux/device.h>
> +#include <linux/thermal.h>
> +
> +struct virtual_thermal_sensor;
> +struct thermal_sensor_data;
> +
> +#if IS_ENABLED(CONFIG_VIRTUAL_THERMAL)
> +struct thermal_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 thermal_sensor_data *sensor_data);
> +struct thermal_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_thermal_sensor *sensor);
> +#else
> +static inline struct thermal_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 thermal_sensor_data *sensor_data)
> +{
> +}
> +
> +static inline struct thermal_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_thermal_sensor *sensor)
> +{
> +}
> +#endif
> +
> +#endif /* __THERMAL_VIRTUAL_SENSOR_H__ */
> diff --git a/drivers/thermal/virtual_thermal_sensor.c b/drivers/thermal/virtual_thermal_sensor.c
> new file mode 100644
> index 0000000000000..234563af6643e
> --- /dev/null
> +++ b/drivers/thermal/virtual_thermal_sensor.c
> @@ -0,0 +1,350 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2021 BayLibre
Please add information on what's in this file to the preamble.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#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-thermal-sensor.h"
> +
> +struct thermal_sensor_data {
> + struct list_head node;
> +
> + /* sensor interface */
> + int id;
> + void *sensor_data;
> + const struct thermal_zone_of_device_ops *ops;
> +};
> +
> +struct virtual_thermal_sensor {
> + int count;
> + struct thermal_sensor_data *sensors;
> + struct thermal_zone_device *tzd;
> + int (*aggr_temp)(int temp1, int temp2);
> +
> + struct list_head node;
> +};
> +
> +static LIST_HEAD(thermal_sensors);
> +static LIST_HEAD(virtual_sensors);
These lists don't seem to be protected against concurrent access and
they should be.
> +
> +static int max_temp(int temp1, int temp2)
> +{
> + return max(temp1, temp2);
> +}
> +
> +static int min_temp(int temp1, int temp2)
> +{
> + return min(temp1, temp2);
> +}
> +
> +static int avg_temp(int temp1, int temp2)
> +{
> + return ((temp1) / 2) + ((temp2) / 2) + (((temp1) % 2 + (temp2) % 2) / 2);
There are a few redundant parens here.
And why not
return (temp1 + temp2) / 2;
> +}
> +
> +static int virtual_thermal_sensor_get_temp(void *data, int *temperature)
> +{
> + struct virtual_thermal_sensor *sensor = data;
> + int max_temp = INT_MIN;
> + int temp;
> + int i;
> +
Some synchronization (eg. locking) should be added to this. It is racy as is.
> + for (i = 0; i < sensor->count; i++) {
> + struct thermal_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 = sensor->aggr_temp(max_temp, temp);
> + }
> +
> + *temperature = max_temp;
> +
> + return 0;
> +}
> +
> +static const struct thermal_zone_of_device_ops virtual_thermal_sensor_ops = {
> + .get_temp = virtual_thermal_sensor_get_temp,
> +};
> +
> +static int virtual_sensor_add_sensor(struct virtual_thermal_sensor *sensor,
> + struct of_phandle_args args,
> + int index)
> +{
> + struct thermal_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));
The data object being copied includes pointers, so I wouldn't copy
them like this. Especially the ops one annotated as const.
> + return 0;
> + }
> + }
> +
> + return -ENODEV;
> +}
> +
> +static int virtual_thermal_sensor_probe(struct platform_device *pdev)
> +{
> + struct virtual_thermal_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,
> + &args);
> + if (ret)
> + return ret;
> +
> + ret = virtual_sensor_add_sensor(sensor, args, i);
> + if (ret)
> + return ret;
> + }
> +
> + ret = of_property_read_u32(dev->of_node, "aggregation-function", &type);
> + if (ret)
> + return ret;
> +
> + switch (type) {
> + case VIRTUAL_THERMAL_SENSOR_MAX:
> + sensor->aggr_temp = max_temp;
> + break;
> + case VIRTUAL_THERMAL_SENSOR_MIN:
> + sensor->aggr_temp = min_temp;
> + break;
> + case VIRTUAL_THERMAL_SENSOR_AVG:
> + sensor->aggr_temp = avg_temp;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + sensor->tzd = devm_thermal_zone_of_sensor_register(dev, 0, sensor,
> + &virtual_thermal_sensor_ops);
> + if (IS_ERR(sensor->tzd))
> + return PTR_ERR(sensor->tzd);
> +
> + platform_set_drvdata(pdev, sensor);
> + list_add(&sensor->node, &virtual_sensors);
> +
> + return 0;
> +}
> +
> +static int virtual_thermal_sensor_remove(struct platform_device *pdev)
> +{
> + struct virtual_thermal_sensor *sensor;
> +
> + sensor = platform_get_drvdata(pdev);
> + list_del(&sensor->node);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id virtual_thermal_sensor_of_match[] = {
> + {
> + .compatible = "virtual,thermal-sensor",
> + },
> + {
> + },
> +};
> +MODULE_DEVICE_TABLE(of, virtual_thermal_sensor_of_match);
> +
> +static struct platform_driver virtual_thermal_sensor = {
> + .probe = virtual_thermal_sensor_probe,
> + .remove = virtual_thermal_sensor_remove,
> + .driver = {
> + .name = "virtual-thermal-sensor",
> + .of_match_table = virtual_thermal_sensor_of_match,
> + },
> +};
> +
> +/**
> + * thermal_virtual_sensor_register - registers a sensor that could be a virtual
> + * 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 sensor
> + * @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.
So who's expected to be calling it?
> + *
> + * The thermal zone temperature is provided by the @get_temp function
> + * pointer. When called, it will have the private pointer @data back.
It is unclear what you mean here. I think you are describing a return
mechanism, but as is it is hard to follow.
> + *
> + * Return: On success returns a valid struct thermal_zone_device,
That doesn't seem to be the case. It returns an address returned by
devm_kzalloc() and the memory at that address is expected to be a
thermal_sensor_data object.
> + * otherwise, it returns a corresponding ERR_PTR(). Caller must
> + * check the return value with help of IS_ERR() helper.
> + */
> +struct thermal_sensor_data *thermal_virtual_sensor_register(
> + struct device *dev, int sensor_id, void *data,
> + const struct thermal_zone_of_device_ops *ops)
> +{
> + struct thermal_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;
Overall, this is non-modular code squeezed into a module.
If this module goes away and whoever called it wants to unregister the
sensor, they will crash, so to a minimum it should do a
try_module_get() on the module containing it.
> +}
> +EXPORT_SYMBOL_GPL(thermal_virtual_sensor_register);
> +
> +/**
> + * thermal_virtual_sensor_unregister - unregisters a sensor
> + * @dev: a valid struct device pointer of a sensor device.
> + * @sensor_data: a pointer to struct thermal_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 thermal_sensor_data *sensor_data)
> +{
> + struct thermal_sensor_data *sd;
> + struct virtual_thermal_sensor *sensor;
> + int i;
> +
All of the code in this function is obviously racy.
> + list_del(&sensor_data->node);
> +
> + list_for_each_entry(sensor, &virtual_sensors, node) {
> + for (i = 0; i < sensor->count; i++) {
> + sd = &sensor->sensors[i];
> + if (sd->id == sensor_data->id &&
> + sd->sensor_data == sensor_data->sensor_data) {
> + sd->ops = NULL;
> + }
The inner braces are not necessary.
> + }
> + }
And again, this code requires the module to be protected from
unloading until the last instance of
thermal_virtual_sensor_unregister() has returned.
> +}
> +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 thermal_sensor_data **)res);
> +}
> +
> +static int devm_thermal_virtual_sensor_match(struct device *dev, void *res,
> + void *data)
> +{
> + struct thermal_sensor_data **r = res;
> +
> + if (WARN_ON(!r || !*r))
> + return 0;
> +
> + return *r == data;
> +}
> +
> +/**
> + * 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 sensor
> + * @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 unbound.
> + */
> +struct thermal_sensor_data *devm_thermal_virtual_sensor_register(
> + struct device *dev, int sensor_id,
> + void *data, const struct thermal_zone_of_device_ops *ops)
> +{
> + struct thermal_sensor_data **ptr, *sensor_data;
> +
> + ptr = devres_alloc(devm_thermal_virtual_sensor_release, sizeof(*ptr),
> + GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + sensor_data = thermal_virtual_sensor_register(dev, sensor_id, data, ops);
> + if (IS_ERR(sensor_data)) {
> + devres_free(ptr);
> + return sensor_data;
> + }
> +
> + *ptr = sensor_data;
> + devres_add(dev, ptr);
> +
> + return sensor_data;
> +}
> +EXPORT_SYMBOL_GPL(devm_thermal_virtual_sensor_register);
> +
> +/**
> + * devm_thermal_virtual_sensor_unregister - Resource managed version of
> + * thermal_virtual_sensor_unregister().
> + * @dev: Device for which resource was allocated.
> + * @sensor: a pointer to struct thermal_zone_device where the sensor is registered.
> + *
> + * This function removes the sensor from the list of sensors registered with
> + * devm_thermal_virtual_sensor_register() API.
> + * Normally this function will not need to be called and the resource
> + * management code will ensure that the resource is freed.
> + */
> +void devm_thermal_virtual_sensor_unregister(struct device *dev,
> + struct virtual_thermal_sensor *sensor)
> +{
> + WARN_ON(devres_release(dev, devm_thermal_virtual_sensor_release,
> + devm_thermal_virtual_sensor_match, sensor));
> +}
> +EXPORT_SYMBOL_GPL(devm_thermal_virtual_sensor_unregister);
> +
> +module_platform_driver(virtual_thermal_sensor);
> +MODULE_AUTHOR("Alexandre Bailon <[email protected]>");
> +MODULE_DESCRIPTION("Virtual thermal sensor");
> +MODULE_LICENSE("GPL v2");
> --
On Thu, Oct 7, 2021 at 6:38 PM Rafael J. Wysocki <[email protected]> wrote:
>
> On Fri, Sep 17, 2021 at 9:25 AM Alexandre Bailon <[email protected]> wrote:
> >
> > This adds a virtual thermal sensor that reads temperature from
>
> This should be "virtual sensor driver" I suppose.
>
> > hardware sensor and return an aggregated temperature.
>
> "returns"
>
> Also wrapping it around a single hardware sensor makes a little sense
> AFAICS, so I guess you mean "multiple hardware sensors" here.
>
> > Currently, this supports three operations:
> > the minimum, maximum and average temperature.
>
> They are aggregation functions rather than "operations"
>
> >
> > Reported-by: kernel test robot <[email protected]>
>
> This is new code, so what does it fix?
>
> > Signed-off-by: Alexandre Bailon <[email protected]>
> > ---
> > drivers/thermal/Kconfig | 8 +
> > drivers/thermal/Makefile | 1 +
> > drivers/thermal/virtual-thermal-sensor.h | 54 ++++
> > drivers/thermal/virtual_thermal_sensor.c | 350 +++++++++++++++++++++++
> > 4 files changed, 413 insertions(+)
> > create mode 100644 drivers/thermal/virtual-thermal-sensor.h
> > create mode 100644 drivers/thermal/virtual_thermal_sensor.c
> >
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index d7f44deab5b1d..20bc93c48f5b1 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -228,6 +228,14 @@ config THERMAL_MMIO
> > register or shared memory, is a potential candidate to work with this
> > driver.
> >
> > +config VIRTUAL_THERMAL
> > + tristate "Virtual thermal sensor driver"
>
> This should be "DT-based virtual thermal sensor driver" I think.
>
> > + depends on THERMAL_OF || COMPILE_TEST
> > + help
> > + This option enables the generic thermal sensor aggregator.
>
> "generic DT-based"
>
> > + This driver creates a thermal sensor that reads the hardware sensors
>
> s/the/multiple/
>
> > + and aggregate the temperature.
>
> "aggregates their output"
>
> > +
> > config HISI_THERMAL
> > tristate "Hisilicon thermal driver"
> > depends on ARCH_HISI || COMPILE_TEST
> > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> > index 82fc3e616e54b..8bf55973059c5 100644
> > --- a/drivers/thermal/Makefile
> > +++ b/drivers/thermal/Makefile
> > @@ -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_thermal_sensor.o
> > diff --git a/drivers/thermal/virtual-thermal-sensor.h b/drivers/thermal/virtual-thermal-sensor.h
> > new file mode 100644
> > index 0000000000000..3bbf7c324dddc
> > --- /dev/null
> > +++ b/drivers/thermal/virtual-thermal-sensor.h
> > @@ -0,0 +1,54 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2021 BayLibre
> > + */
> > +
> > +#ifndef __THERMAL_VIRTUAL_SENSOR_H__
> > +#define __THERMAL_VIRTUAL_SENSOR_H__
> > +
> > +#include <linux/device.h>
> > +#include <linux/thermal.h>
> > +
> > +struct virtual_thermal_sensor;
> > +struct thermal_sensor_data;
> > +
> > +#if IS_ENABLED(CONFIG_VIRTUAL_THERMAL)
> > +struct thermal_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 thermal_sensor_data *sensor_data);
> > +struct thermal_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_thermal_sensor *sensor);
> > +#else
> > +static inline struct thermal_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 thermal_sensor_data *sensor_data)
> > +{
> > +}
> > +
> > +static inline struct thermal_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_thermal_sensor *sensor)
> > +{
> > +}
> > +#endif
> > +
> > +#endif /* __THERMAL_VIRTUAL_SENSOR_H__ */
> > diff --git a/drivers/thermal/virtual_thermal_sensor.c b/drivers/thermal/virtual_thermal_sensor.c
> > new file mode 100644
> > index 0000000000000..234563af6643e
> > --- /dev/null
> > +++ b/drivers/thermal/virtual_thermal_sensor.c
> > @@ -0,0 +1,350 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2021 BayLibre
>
> Please add information on what's in this file to the preamble.
>
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/export.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_platform.h>
> > +#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-thermal-sensor.h"
> > +
> > +struct thermal_sensor_data {
> > + struct list_head node;
> > +
> > + /* sensor interface */
> > + int id;
> > + void *sensor_data;
> > + const struct thermal_zone_of_device_ops *ops;
> > +};
> > +
> > +struct virtual_thermal_sensor {
> > + int count;
> > + struct thermal_sensor_data *sensors;
> > + struct thermal_zone_device *tzd;
> > + int (*aggr_temp)(int temp1, int temp2);
> > +
> > + struct list_head node;
> > +};
> > +
> > +static LIST_HEAD(thermal_sensors);
> > +static LIST_HEAD(virtual_sensors);
>
> These lists don't seem to be protected against concurrent access and
> they should be.
>
> > +
> > +static int max_temp(int temp1, int temp2)
> > +{
> > + return max(temp1, temp2);
> > +}
> > +
> > +static int min_temp(int temp1, int temp2)
> > +{
> > + return min(temp1, temp2);
> > +}
> > +
> > +static int avg_temp(int temp1, int temp2)
> > +{
> > + return ((temp1) / 2) + ((temp2) / 2) + (((temp1) % 2 + (temp2) % 2) / 2);
>
> There are a few redundant parens here.
>
> And why not
>
> return (temp1 + temp2) / 2;
>
> > +}
> > +
> > +static int virtual_thermal_sensor_get_temp(void *data, int *temperature)
> > +{
> > + struct virtual_thermal_sensor *sensor = data;
> > + int max_temp = INT_MIN;
> > + int temp;
> > + int i;
> > +
>
> Some synchronization (eg. locking) should be added to this. It is racy as is.
>
> > + for (i = 0; i < sensor->count; i++) {
> > + struct thermal_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 = sensor->aggr_temp(max_temp, temp);
> > + }
> > +
> > + *temperature = max_temp;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct thermal_zone_of_device_ops virtual_thermal_sensor_ops = {
> > + .get_temp = virtual_thermal_sensor_get_temp,
> > +};
> > +
> > +static int virtual_sensor_add_sensor(struct virtual_thermal_sensor *sensor,
> > + struct of_phandle_args args,
> > + int index)
> > +{
> > + struct thermal_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));
>
> The data object being copied includes pointers, so I wouldn't copy
> them like this. Especially the ops one annotated as const.
>
> > + return 0;
> > + }
> > + }
> > +
> > + return -ENODEV;
> > +}
> > +
> > +static int virtual_thermal_sensor_probe(struct platform_device *pdev)
> > +{
> > + struct virtual_thermal_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,
> > + &args);
> > + if (ret)
> > + return ret;
> > +
> > + ret = virtual_sensor_add_sensor(sensor, args, i);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + ret = of_property_read_u32(dev->of_node, "aggregation-function", &type);
> > + if (ret)
> > + return ret;
> > +
> > + switch (type) {
> > + case VIRTUAL_THERMAL_SENSOR_MAX:
> > + sensor->aggr_temp = max_temp;
> > + break;
> > + case VIRTUAL_THERMAL_SENSOR_MIN:
> > + sensor->aggr_temp = min_temp;
> > + break;
> > + case VIRTUAL_THERMAL_SENSOR_AVG:
> > + sensor->aggr_temp = avg_temp;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + sensor->tzd = devm_thermal_zone_of_sensor_register(dev, 0, sensor,
> > + &virtual_thermal_sensor_ops);
> > + if (IS_ERR(sensor->tzd))
> > + return PTR_ERR(sensor->tzd);
> > +
> > + platform_set_drvdata(pdev, sensor);
> > + list_add(&sensor->node, &virtual_sensors);
> > +
> > + return 0;
> > +}
> > +
> > +static int virtual_thermal_sensor_remove(struct platform_device *pdev)
> > +{
> > + struct virtual_thermal_sensor *sensor;
> > +
> > + sensor = platform_get_drvdata(pdev);
> > + list_del(&sensor->node);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id virtual_thermal_sensor_of_match[] = {
> > + {
> > + .compatible = "virtual,thermal-sensor",
> > + },
> > + {
> > + },
> > +};
> > +MODULE_DEVICE_TABLE(of, virtual_thermal_sensor_of_match);
> > +
> > +static struct platform_driver virtual_thermal_sensor = {
> > + .probe = virtual_thermal_sensor_probe,
> > + .remove = virtual_thermal_sensor_remove,
> > + .driver = {
> > + .name = "virtual-thermal-sensor",
> > + .of_match_table = virtual_thermal_sensor_of_match,
> > + },
> > +};
> > +
> > +/**
> > + * thermal_virtual_sensor_register - registers a sensor that could be a virtual
> > + * 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 sensor
> > + * @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.
>
> So who's expected to be calling it?
>
> > + *
> > + * The thermal zone temperature is provided by the @get_temp function
> > + * pointer. When called, it will have the private pointer @data back.
>
> It is unclear what you mean here. I think you are describing a return
> mechanism, but as is it is hard to follow.
>
> > + *
> > + * Return: On success returns a valid struct thermal_zone_device,
>
> That doesn't seem to be the case. It returns an address returned by
> devm_kzalloc() and the memory at that address is expected to be a
> thermal_sensor_data object.
>
> > + * otherwise, it returns a corresponding ERR_PTR(). Caller must
> > + * check the return value with help of IS_ERR() helper.
> > + */
> > +struct thermal_sensor_data *thermal_virtual_sensor_register(
> > + struct device *dev, int sensor_id, void *data,
> > + const struct thermal_zone_of_device_ops *ops)
> > +{
> > + struct thermal_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;
>
> Overall, this is non-modular code squeezed into a module.
>
> If this module goes away and whoever called it wants to unregister the
> sensor, they will crash, so to a minimum it should do a
> try_module_get() on the module containing it.
And that wouldn't be sufficient either, because in theory the module
may go away before that try_module_get() gets called. Sorry for the
confusion.
On Thu, Oct 7, 2021 at 6:38 PM Rafael J. Wysocki <[email protected]> wrote:
[cut]
> > +static int virtual_thermal_sensor_probe(struct platform_device *pdev)
> > +{
> > + struct virtual_thermal_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,
> > + &args);
> > + if (ret)
> > + return ret;
> > +
> > + ret = virtual_sensor_add_sensor(sensor, args, i);
> > + if (ret)
> > + return ret;
One more issue that escaped me earlier is that if the hardware sensor
being looked for is not there in the thermal_sensors list at this
point (say because it has not been added to it yet by whoever is
expected to do that), the above will return -ENODEV and all probe will
fail without a way to retry.
It would be better to return -EPROBE_DEFER from here IIUC.
However, this still wouldn't address a more general issue that if the
hardware sensor gets unregistered and then registered again (eg. by
unloading and reloading a module managing it), it will not be added
back to the corresponding virtual sensor's data set. This should not
be very hard to address and it would take care of the above
initialization ordering issue automatically.
Hi Rafael,
On 10/7/21 6:38 PM, Rafael J. Wysocki wrote:
> On Fri, Sep 17, 2021 at 9:25 AM Alexandre Bailon <[email protected]> wrote:
>> This adds a virtual thermal sensor that reads temperature from
> This should be "virtual sensor driver" I suppose.
>
>> hardware sensor and return an aggregated temperature.
> "returns"
>
> Also wrapping it around a single hardware sensor makes a little sense
> AFAICS, so I guess you mean "multiple hardware sensors" here.
>
>> Currently, this supports three operations:
>> the minimum, maximum and average temperature.
> They are aggregation functions rather than "operations"
>
>> Reported-by: kernel test robot <[email protected]>
> This is new code, so what does it fix?
The CI has found some build issues.
I was not sure if I had to add this or not.
>
>> Signed-off-by: Alexandre Bailon <[email protected]>
>> ---
>> drivers/thermal/Kconfig | 8 +
>> drivers/thermal/Makefile | 1 +
>> drivers/thermal/virtual-thermal-sensor.h | 54 ++++
>> drivers/thermal/virtual_thermal_sensor.c | 350 +++++++++++++++++++++++
>> 4 files changed, 413 insertions(+)
>> create mode 100644 drivers/thermal/virtual-thermal-sensor.h
>> create mode 100644 drivers/thermal/virtual_thermal_sensor.c
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index d7f44deab5b1d..20bc93c48f5b1 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -228,6 +228,14 @@ config THERMAL_MMIO
>> register or shared memory, is a potential candidate to work with this
>> driver.
>>
>> +config VIRTUAL_THERMAL
>> + tristate "Virtual thermal sensor driver"
> This should be "DT-based virtual thermal sensor driver" I think.
>
>> + depends on THERMAL_OF || COMPILE_TEST
>> + help
>> + This option enables the generic thermal sensor aggregator.
> "generic DT-based"
>
>> + This driver creates a thermal sensor that reads the hardware sensors
> s/the/multiple/
>
>> + and aggregate the temperature.
> "aggregates their output"
>
>> +
>> config HISI_THERMAL
>> tristate "Hisilicon thermal driver"
>> depends on ARCH_HISI || COMPILE_TEST
>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>> index 82fc3e616e54b..8bf55973059c5 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -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_thermal_sensor.o
>> diff --git a/drivers/thermal/virtual-thermal-sensor.h b/drivers/thermal/virtual-thermal-sensor.h
>> new file mode 100644
>> index 0000000000000..3bbf7c324dddc
>> --- /dev/null
>> +++ b/drivers/thermal/virtual-thermal-sensor.h
>> @@ -0,0 +1,54 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2021 BayLibre
>> + */
>> +
>> +#ifndef __THERMAL_VIRTUAL_SENSOR_H__
>> +#define __THERMAL_VIRTUAL_SENSOR_H__
>> +
>> +#include <linux/device.h>
>> +#include <linux/thermal.h>
>> +
>> +struct virtual_thermal_sensor;
>> +struct thermal_sensor_data;
>> +
>> +#if IS_ENABLED(CONFIG_VIRTUAL_THERMAL)
>> +struct thermal_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 thermal_sensor_data *sensor_data);
>> +struct thermal_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_thermal_sensor *sensor);
>> +#else
>> +static inline struct thermal_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 thermal_sensor_data *sensor_data)
>> +{
>> +}
>> +
>> +static inline struct thermal_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_thermal_sensor *sensor)
>> +{
>> +}
>> +#endif
>> +
>> +#endif /* __THERMAL_VIRTUAL_SENSOR_H__ */
>> diff --git a/drivers/thermal/virtual_thermal_sensor.c b/drivers/thermal/virtual_thermal_sensor.c
>> new file mode 100644
>> index 0000000000000..234563af6643e
>> --- /dev/null
>> +++ b/drivers/thermal/virtual_thermal_sensor.c
>> @@ -0,0 +1,350 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2021 BayLibre
> Please add information on what's in this file to the preamble.
Does something like "DT-based virtual thermal sensor driver" would be
enough,
or should I details a little bit more ?
>
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/export.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_platform.h>
>> +#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-thermal-sensor.h"
>> +
>> +struct thermal_sensor_data {
>> + struct list_head node;
>> +
>> + /* sensor interface */
>> + int id;
>> + void *sensor_data;
>> + const struct thermal_zone_of_device_ops *ops;
>> +};
>> +
>> +struct virtual_thermal_sensor {
>> + int count;
>> + struct thermal_sensor_data *sensors;
>> + struct thermal_zone_device *tzd;
>> + int (*aggr_temp)(int temp1, int temp2);
>> +
>> + struct list_head node;
>> +};
>> +
>> +static LIST_HEAD(thermal_sensors);
>> +static LIST_HEAD(virtual_sensors);
> These lists don't seem to be protected against concurrent access and
> they should be.
I Reworked the driver and I am going to remove them.
>
>> +
>> +static int max_temp(int temp1, int temp2)
>> +{
>> + return max(temp1, temp2);
>> +}
>> +
>> +static int min_temp(int temp1, int temp2)
>> +{
>> + return min(temp1, temp2);
>> +}
>> +
>> +static int avg_temp(int temp1, int temp2)
>> +{
>> + return ((temp1) / 2) + ((temp2) / 2) + (((temp1) % 2 + (temp2) % 2) / 2);
> There are a few redundant parens here.
>
> And why not
>
> return (temp1 + temp2) / 2;
>
>> +}
>> +
>> +static int virtual_thermal_sensor_get_temp(void *data, int *temperature)
>> +{
>> + struct virtual_thermal_sensor *sensor = data;
>> + int max_temp = INT_MIN;
>> + int temp;
>> + int i;
>> +
> Some synchronization (eg. locking) should be added to this. It is racy as is.
>
I am not sure to figure out what is racy here.
Anyway, I will update the logic of the driver and simplify this
function, so this would
probably be fixed.
>> + for (i = 0; i < sensor->count; i++) {
>> + struct thermal_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 = sensor->aggr_temp(max_temp, temp);
>> + }
>> +
>> + *temperature = max_temp;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct thermal_zone_of_device_ops virtual_thermal_sensor_ops = {
>> + .get_temp = virtual_thermal_sensor_get_temp,
>> +};
>> +
>> +static int virtual_sensor_add_sensor(struct virtual_thermal_sensor *sensor,
>> + struct of_phandle_args args,
>> + int index)
>> +{
>> + struct thermal_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));
> The data object being copied includes pointers, so I wouldn't copy
> them like this. Especially the ops one annotated as const.
>
>> + return 0;
>> + }
>> + }
>> +
>> + return -ENODEV;
>> +}
>> +
>> +static int virtual_thermal_sensor_probe(struct platform_device *pdev)
>> +{
>> + struct virtual_thermal_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,
>> + &args);
>> + if (ret)
>> + return ret;
>> +
>> + ret = virtual_sensor_add_sensor(sensor, args, i);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + ret = of_property_read_u32(dev->of_node, "aggregation-function", &type);
>> + if (ret)
>> + return ret;
>> +
>> + switch (type) {
>> + case VIRTUAL_THERMAL_SENSOR_MAX:
>> + sensor->aggr_temp = max_temp;
>> + break;
>> + case VIRTUAL_THERMAL_SENSOR_MIN:
>> + sensor->aggr_temp = min_temp;
>> + break;
>> + case VIRTUAL_THERMAL_SENSOR_AVG:
>> + sensor->aggr_temp = avg_temp;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + sensor->tzd = devm_thermal_zone_of_sensor_register(dev, 0, sensor,
>> + &virtual_thermal_sensor_ops);
>> + if (IS_ERR(sensor->tzd))
>> + return PTR_ERR(sensor->tzd);
>> +
>> + platform_set_drvdata(pdev, sensor);
>> + list_add(&sensor->node, &virtual_sensors);
>> +
>> + return 0;
>> +}
>> +
>> +static int virtual_thermal_sensor_remove(struct platform_device *pdev)
>> +{
>> + struct virtual_thermal_sensor *sensor;
>> +
>> + sensor = platform_get_drvdata(pdev);
>> + list_del(&sensor->node);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id virtual_thermal_sensor_of_match[] = {
>> + {
>> + .compatible = "virtual,thermal-sensor",
>> + },
>> + {
>> + },
>> +};
>> +MODULE_DEVICE_TABLE(of, virtual_thermal_sensor_of_match);
>> +
>> +static struct platform_driver virtual_thermal_sensor = {
>> + .probe = virtual_thermal_sensor_probe,
>> + .remove = virtual_thermal_sensor_remove,
>> + .driver = {
>> + .name = "virtual-thermal-sensor",
>> + .of_match_table = virtual_thermal_sensor_of_match,
>> + },
>> +};
>> +
>> +/**
>> + * thermal_virtual_sensor_register - registers a sensor that could be a virtual
>> + * 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 sensor
>> + * @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.
> So who's expected to be calling it?
Any driver that would like to expose its sensors to the virtual thermal
sensor driver,
which was a bad solution ...
In the next series, I will remove this function, as well the other
register / unregister
functions, and use directly thermal_zone_device to get the temperature
from the sensors.
Probe will take care of finding the thermal_zone_device listed in the
virtual sensor device
tree node, will defer if a thermal_zone_device is not yet ready and use
try_module_get()
to prevent any thermal_zone_device used by the virtual sensor to be removed.
Best Regards,
Alexandre
>
>> + *
>> + * The thermal zone temperature is provided by the @get_temp function
>> + * pointer. When called, it will have the private pointer @data back.
> It is unclear what you mean here. I think you are describing a return
> mechanism, but as is it is hard to follow.
>
>> + *
>> + * Return: On success returns a valid struct thermal_zone_device,
> That doesn't seem to be the case. It returns an address returned by
> devm_kzalloc() and the memory at that address is expected to be a
> thermal_sensor_data object.
>
>> + * otherwise, it returns a corresponding ERR_PTR(). Caller must
>> + * check the return value with help of IS_ERR() helper.
>> + */
>> +struct thermal_sensor_data *thermal_virtual_sensor_register(
>> + struct device *dev, int sensor_id, void *data,
>> + const struct thermal_zone_of_device_ops *ops)
>> +{
>> + struct thermal_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;
> Overall, this is non-modular code squeezed into a module.
>
> If this module goes away and whoever called it wants to unregister the
> sensor, they will crash, so to a minimum it should do a
> try_module_get() on the module containing it.
>
>> +}
>> +EXPORT_SYMBOL_GPL(thermal_virtual_sensor_register);
>> +
>> +/**
>> + * thermal_virtual_sensor_unregister - unregisters a sensor
>> + * @dev: a valid struct device pointer of a sensor device.
>> + * @sensor_data: a pointer to struct thermal_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 thermal_sensor_data *sensor_data)
>> +{
>> + struct thermal_sensor_data *sd;
>> + struct virtual_thermal_sensor *sensor;
>> + int i;
>> +
> All of the code in this function is obviously racy.
>
>> + list_del(&sensor_data->node);
>> +
>> + list_for_each_entry(sensor, &virtual_sensors, node) {
>> + for (i = 0; i < sensor->count; i++) {
>> + sd = &sensor->sensors[i];
>> + if (sd->id == sensor_data->id &&
>> + sd->sensor_data == sensor_data->sensor_data) {
>> + sd->ops = NULL;
>> + }
> The inner braces are not necessary.
>
>> + }
>> + }
> And again, this code requires the module to be protected from
> unloading until the last instance of
> thermal_virtual_sensor_unregister() has returned.
>
>> +}
>> +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 thermal_sensor_data **)res);
>> +}
>> +
>> +static int devm_thermal_virtual_sensor_match(struct device *dev, void *res,
>> + void *data)
>> +{
>> + struct thermal_sensor_data **r = res;
>> +
>> + if (WARN_ON(!r || !*r))
>> + return 0;
>> +
>> + return *r == data;
>> +}
>> +
>> +/**
>> + * 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 sensor
>> + * @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 unbound.
>> + */
>> +struct thermal_sensor_data *devm_thermal_virtual_sensor_register(
>> + struct device *dev, int sensor_id,
>> + void *data, const struct thermal_zone_of_device_ops *ops)
>> +{
>> + struct thermal_sensor_data **ptr, *sensor_data;
>> +
>> + ptr = devres_alloc(devm_thermal_virtual_sensor_release, sizeof(*ptr),
>> + GFP_KERNEL);
>> + if (!ptr)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + sensor_data = thermal_virtual_sensor_register(dev, sensor_id, data, ops);
>> + if (IS_ERR(sensor_data)) {
>> + devres_free(ptr);
>> + return sensor_data;
>> + }
>> +
>> + *ptr = sensor_data;
>> + devres_add(dev, ptr);
>> +
>> + return sensor_data;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_thermal_virtual_sensor_register);
>> +
>> +/**
>> + * devm_thermal_virtual_sensor_unregister - Resource managed version of
>> + * thermal_virtual_sensor_unregister().
>> + * @dev: Device for which resource was allocated.
>> + * @sensor: a pointer to struct thermal_zone_device where the sensor is registered.
>> + *
>> + * This function removes the sensor from the list of sensors registered with
>> + * devm_thermal_virtual_sensor_register() API.
>> + * Normally this function will not need to be called and the resource
>> + * management code will ensure that the resource is freed.
>> + */
>> +void devm_thermal_virtual_sensor_unregister(struct device *dev,
>> + struct virtual_thermal_sensor *sensor)
>> +{
>> + WARN_ON(devres_release(dev, devm_thermal_virtual_sensor_release,
>> + devm_thermal_virtual_sensor_match, sensor));
>> +}
>> +EXPORT_SYMBOL_GPL(devm_thermal_virtual_sensor_unregister);
>> +
>> +module_platform_driver(virtual_thermal_sensor);
>> +MODULE_AUTHOR("Alexandre Bailon <[email protected]>");
>> +MODULE_DESCRIPTION("Virtual thermal sensor");
>> +MODULE_LICENSE("GPL v2");
>> --