Messages in this thread | | | Subject | Re: [RFC PATCH 2/2] thermal: add a virtual sensor to aggregate temperatures | From | Daniel Lezcano <> | Date | Fri, 20 Aug 2021 14:52:33 +0200 |
| |
On 19/08/2021 14:32, Alexandre Bailon wrote: > 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 <abailon@baylibre.com> > --- > drivers/thermal/Kconfig | 8 ++ > drivers/thermal/Makefile | 1 + > drivers/thermal/thermal_aggregator.c | 134 +++++++++++++++++++++++++++ > 3 files changed, 143 insertions(+) > create mode 100644 drivers/thermal/thermal_aggregator.c > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > index 7a4ba50ba97d0..f9c152cfb95bc 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 THERMAL_AGGREGATOR
We discussed the virtual sensor doing aggregation but may be we can give it another purpose in the future like returning a constant temp or low/high pass filter.
It may make sense to use a more generic name like virtual sensor for example.
> + tristate "Generic thermal aggregator driver" > + depends on TERMAL_OF || COMPILE_TEST
s/TERMAL_OF/THERMAL_OF/
> + 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 9729a2b089919..5b20ef15aebbe 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_THERMAL_AGGREGATOR) += thermal_aggregator.o > diff --git a/drivers/thermal/thermal_aggregator.c b/drivers/thermal/thermal_aggregator.c > new file mode 100644 > index 0000000000000..76f871dbfee9e > --- /dev/null > +++ b/drivers/thermal/thermal_aggregator.c > @@ -0,0 +1,134 @@ > +#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> > + > +const char *aggr_types[] = { > + "min", > + "max", > + "avg", > +}; > + > +struct thermal_aggr; > + > +typedef int (*aggregate_fn)(struct thermal_aggr *aggr); > + > +struct thermal_aggr_sensor { > + struct thermal_sensor *sensor; > + > + struct list_head node; > +}; > + > +struct thermal_aggr { > + struct list_head sensors; > + aggregate_fn *aggregate; > + //struct thermal_zone_device *tz; > +}; > + > +static int thermal_aggr_read_temp(void *data, int *temperature) > +{ > + struct thermal_aggr *aggr = data; > + struct thermal_aggr_sensor *sensor; > + int max_temp = 0; > + int temp; > +
What happens if a hardware sensor module is unloaded ?
> + list_for_each_entry(sensor, &aggr->sensors, node) { > + if (!sensor->sensor) { > + return -ENODEV; > + }
> + sensor->sensor->ops->get_temp(sensor->sensor->sensor_data, &temp); > + max_temp = max(max_temp, temp); > + } > + > + *temperature = max_temp; > + > + return 0; > +} > + > +static const struct thermal_zone_of_device_ops thermal_aggr_max_ops = { > + .get_temp = thermal_aggr_read_temp,
.get_temp = thermal_virtual_sensor_get_temp ? > +}; > + > +static int thermal_aggr_probe(struct platform_device *pdev) > +{ > + struct thermal_aggr *aggr; > + struct device *dev = &pdev->dev; > + struct of_phandle_args args; > + int count; > + int ret; > + int i; > + > + count = of_count_phandle_with_args(dev->of_node, > + "thermal-sensors", > + "#thermal-sensor-cells"); > + if (count <= 0) > + return -EINVAL; > + > + aggr = kzalloc(sizeof(*aggr), GFP_KERNEL);
devm_kzalloc
> + INIT_LIST_HEAD(&aggr->sensors); > + > + for (i = 0; i < count; i++) { > + struct thermal_sensor *sensor; > + struct thermal_aggr_sensor *aggr_sensor; > + int id; > + > + ret = of_parse_phandle_with_args(dev->of_node, > + "thermal-sensors", > + "#thermal-sensor-cells", > + i, > + &args); > + if (ret) { > + return ret; > + } > + > + id = args.args_count ? args.args[0] : 0; > + sensor = thermal_of_get_sensor(args.np, id); > + if (sensor == NULL) { > + return -EPROBE_DEFER; > + } > + > + aggr_sensor = kzalloc(sizeof(*aggr_sensor), GFP_KERNEL);
devm_kzalloc
> + aggr_sensor->np = args.np;
Why the 'np' and id are stored, they won't be needed anymore, no ?
> + aggr_sensor->id = id; > + aggr_sensor->sensor = sensor; > + list_add(&aggr_sensor->node, &aggr->sensors); > + } > + > + /*tzdev = */devm_thermal_zone_of_sensor_register(dev, 0, aggr, &thermal_aggr_max_ops); > + > + return 0; > +} > + > +static int thermal_aggr_remove(struct platform_device *pdev) > +{ > + return 0; > +} > + > +static const struct of_device_id thermal_aggr_of_match[] = { > + { > + .compatible = "generic,thermal-aggregator", "^virtual,.*":
As stated in the documentation Documentation/devicetree/bindings/vendor-prefixes.yaml
"^virtual,.*": description: Used for virtual device without specific vendor.
I suggest something like:
.compatible = "virtual,thermal-sensor",
> + }, > + { > + }, > +}; > +MODULE_DEVICE_TABLE(of, thermal_aggr_of_match); > + > +static struct platform_driver thermal_aggr = { > + .probe = thermal_aggr_probe, > + .remove = thermal_aggr_remove, > + .driver = { > + .name = "thermal-aggregator", > + .of_match_table = thermal_aggr_of_match, > + }, > +}; > + > +module_platform_driver(thermal_aggr); > +MODULE_AUTHOR("Alexandre Bailon <abailon@baylibre.com>"); > +MODULE_DESCRIPTION("Thermal sensor aggregator"); > +MODULE_LICENSE("GPL v2"); >
-- <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
| |