Re: [RFC PATCH 0/2] Add a generic virtual thermal sensor

[Alexandre Bailon]

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@xxxxxxxxxxxx/
> >
> > 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