| Message ID | 20210819123215.591593-1-abailon@baylibre.com |
|---|---|
| Headers | show |
| Series | Add a generic virtual thermal sensor | expand |
Hi Alexandre, thanks for the proposal. On 19/08/2021 14:32, Alexandre Bailon wrote: > This series add a virtual thermal sensor that uses the hardware thermal sensors, > aggregate them to return a temperature. > > My first aptempt was to do the aggregation in the thermal zone but it was not > that easy to do, and, there were some case that would have been conflictual > such as setting differents trip for a regular zone and a multisensor zone. > > Instead, I made a virtual thermal sensor that could registered in a thermal > zone, and have its own properties. > It could be added in the device tree, with the list of sensors to aggregate, > and the type of aggregation to be done. > > As example: > soc_max_sensor: soc_max_sensor { > compatible = "generic,thermal-aggregator"; > #thermal-sensor-cells = <1>; > type = "max"; > thermal-sensors = <&lvts 0>, <&lvts 1>, <&lvts 2>, <&lvts 3>, > <&lvts 4>, <&lvts 5>, <&lvts 6>, <&lvts 7>, > <&lvts 8>, <&lvts 9>, <&lvts 10>, <&lvts 11>, > <&lvts 12>, <&lvts 13>, <&lvts 14>, <&lvts 15>, > <&lvts 16>; > }; > > The current series build and work but it would require to be completed > aswell a lot of cleanup. > Before working on it, I would like to get some feedback and I know if that > would an acceptable solution and continue that way. Yes, I think it is going to the right direction. IMO, we can get rid of the thermal_of changes. From a design PoV, the patch itself should be the virtual thermal driver without any changes in the core code, including thermal_of. I have some comments on patch 2/2 > Follows the following discussion: > https://patchwork.kernel.org/project/linux-mediatek/patch/20210617114707.10618-3-ben.tseng@mediatek.com/ > > Alexandre Bailon (2): > thermal: provide a way to get thermal sensor from a device tree node > thermal: add a virtual sensor to aggregate temperatures > > drivers/thermal/Kconfig | 8 ++ > drivers/thermal/Makefile | 1 + > drivers/thermal/thermal_aggregator.c | 134 +++++++++++++++++++++++++++ > drivers/thermal/thermal_of.c | 43 +++++++++ > include/linux/thermal.h | 12 +++ > 5 files changed, 198 insertions(+) > create mode 100644 drivers/thermal/thermal_aggregator.c > -- <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
Hi Daniel, On 20/08/2021 13:30, Daniel Lezcano wrote: > Hi Alexandre, > > thanks for the proposal. > > On 19/08/2021 14:32, Alexandre Bailon wrote: >> This series add a virtual thermal sensor that uses the hardware thermal sensors, >> aggregate them to return a temperature. >> >> My first aptempt was to do the aggregation in the thermal zone but it was not >> that easy to do, and, there were some case that would have been conflictual >> such as setting differents trip for a regular zone and a multisensor zone. >> >> Instead, I made a virtual thermal sensor that could registered in a thermal >> zone, and have its own properties. >> It could be added in the device tree, with the list of sensors to aggregate, >> and the type of aggregation to be done. >> >> As example: >> soc_max_sensor: soc_max_sensor { >> compatible = "generic,thermal-aggregator"; >> #thermal-sensor-cells = <1>; >> type = "max"; >> thermal-sensors = <&lvts 0>, <&lvts 1>, <&lvts 2>, <&lvts 3>, >> <&lvts 4>, <&lvts 5>, <&lvts 6>, <&lvts 7>, >> <&lvts 8>, <&lvts 9>, <&lvts 10>, <&lvts 11>, >> <&lvts 12>, <&lvts 13>, <&lvts 14>, <&lvts 15>, >> <&lvts 16>; >> }; >> >> The current series build and work but it would require to be completed >> aswell a lot of cleanup. >> Before working on it, I would like to get some feedback and I know if that >> would an acceptable solution and continue that way. > Yes, I think it is going to the right direction. > > IMO, we can get rid of the thermal_of changes. From a design PoV, the > patch itself should be the virtual thermal driver without any changes in > the core code, including thermal_of. I made that changes in order to be able to get the hw sensors from the virtual sensor. I am not really satisfied of that patch but that the simplest way I found to do it. How would you proceed to get the hw sensor from its device tree phandle and id ? Thanks, Alexandre > > I have some comments on patch 2/2 > > >> Follows the following discussion: >> https://patchwork.kernel.org/project/linux-mediatek/patch/20210617114707.10618-3-ben.tseng@mediatek.com/ >> >> Alexandre Bailon (2): >> thermal: provide a way to get thermal sensor from a device tree node >> thermal: add a virtual sensor to aggregate temperatures >> >> drivers/thermal/Kconfig | 8 ++ >> drivers/thermal/Makefile | 1 + >> drivers/thermal/thermal_aggregator.c | 134 +++++++++++++++++++++++++++ >> drivers/thermal/thermal_of.c | 43 +++++++++ >> include/linux/thermal.h | 12 +++ >> 5 files changed, 198 insertions(+) >> create mode 100644 drivers/thermal/thermal_aggregator.c >> >
On 23/08/2021 09:35, Alexandre Bailon wrote: [ ... ] > I am not really satisfied of that patch but that the simplest way I > found to do it. > How would you proceed to get the hw sensor from its device tree phandle > and id ? Could the function 'thermal_zone_of_get_sensor_id' help ? -- <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
This series add a virtual thermal sensor that uses the hardware thermal sensors, aggregate them to return a temperature. My first aptempt was to do the aggregation in the thermal zone but it was not that easy to do, and, there were some case that would have been conflictual such as setting differents trip for a regular zone and a multisensor zone. Instead, I made a virtual thermal sensor that could registered in a thermal zone, and have its own properties. It could be added in the device tree, with the list of sensors to aggregate, and the type of aggregation to be done. As example: soc_max_sensor: soc_max_sensor { compatible = "generic,thermal-aggregator"; #thermal-sensor-cells = <1>; type = "max"; thermal-sensors = <&lvts 0>, <&lvts 1>, <&lvts 2>, <&lvts 3>, <&lvts 4>, <&lvts 5>, <&lvts 6>, <&lvts 7>, <&lvts 8>, <&lvts 9>, <&lvts 10>, <&lvts 11>, <&lvts 12>, <&lvts 13>, <&lvts 14>, <&lvts 15>, <&lvts 16>; }; The current series build and work but it would require to be completed aswell a lot of cleanup. Before working on it, I would like to get some feedback and I know if that would an acceptable solution and continue that way. Follows the following discussion: https://patchwork.kernel.org/project/linux-mediatek/patch/20210617114707.10618-3-ben.tseng@mediatek.com/ Alexandre Bailon (2): thermal: provide a way to get thermal sensor from a device tree node thermal: add a virtual sensor to aggregate temperatures drivers/thermal/Kconfig | 8 ++ drivers/thermal/Makefile | 1 + drivers/thermal/thermal_aggregator.c | 134 +++++++++++++++++++++++++++ drivers/thermal/thermal_of.c | 43 +++++++++ include/linux/thermal.h | 12 +++ 5 files changed, 198 insertions(+) create mode 100644 drivers/thermal/thermal_aggregator.c