Re: [RFC PATCH 3/4] thermal: ti-soc-thermal: use thermal DT infrastructure

[Eduardo Valentin <eduardo.valentin@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 5688 bytes --]

On 15-07-2013 08:59, Lucas Stach wrote:
> Am Montag, den 15.07.2013, 08:33 -0400 schrieb Eduardo Valentin:
>> On 15-07-2013 08:12, Lucas Stach wrote:
>>> Hi Eduardo and others,
>>>
>>> Eduardo Valentin <eduardo.valentin <at> ti.com> writes:
>>>
>>>>
>>>> This patch improves the ti-soc-thermal driver by adding the
>>>> support to build the thermal zones based on DT nodes.
>>>>
>>>> The driver will have two options now to build the thermal
>>>> zones. The first option is the zones originally coded
>>>> in this driver. So, the driver behavior will be same
>>>> if there is no DT node describing the zones. The second
>>>> option, when it is found a DT node with thermal data,
>>>> will used the common infrastructure to build the thermal
>>>> zone and bind its cooling devices.
>>>>
>>>> In either case, this driver still adds to the system
>>>> a cpufreq cooling device.
>>>>
>>>
>>> I really like the idea to configure thermal zones using devicetree, it's a
>>> step in the right direction. We might follow suit with the i.MX6 tempmon
>>> driver to use this infrastructure.
>>>
>>> What I strongly dislike is the notion of the sensor drivers instantiating
>>> the cooling devices and the resulting devicetree binding. This seems really
>>> backward to me.
>>> I would rather see the drivers associated with the cooling devices (like
>>> cpufreq and the respective gpu drivers) to instantiate the cooling devices
>>> and the thermal zone referring to them through phandles. I think it
>>> shouldn't be too much work to go in that direction.
>>> The current method might be enough to work with the current thermal platform
>>> drivers, but if you want to go down the route to eventually use plain i2c
>>> devices (likely with an existing hwmon driver) you have to get away from the
>>> sensor devices instantiating the cooling devices.
>>
>> I agree with you. While implementing the RFC, it looks awkward that the
>> ti-soc-thermal driver had to do the job to load the cpufreq-cooling
>> device. Problem is that a cooling device is not really a real device,
>> and might get flamed while represented as a device tree node.
>>
> I don't think we even need to represent this into the device tree. We
> already have the CPU nodes there and the cpu-freq driver is already
> linked to those. It should be easy to have a global list of registered
> thermal devices in the thermal framework together with their associated
> devices, so one could look up cooling devices through the thermal
> framework when we only have a phandle to the cpu node.

Well, we do have a list of thermal cooling devices associated with its
corresponding struct device. That is private data to the thermal
framework. However, one needs to load the cooling device in order to its
data structure appear in this list. The framework can not be proactive
here. Some other entity needs to see the need and inform the thermal
framework of it.

For instance, assuming that all systems will need a cpufreq cooling
device is a flaw, because that is not the case. Thus, it makes sense to
have a property, say at the cpu node, to determine that it needs
cooling. However, that won't be saying how it would cool off.


> 
>> I could try to push something following the same idea as the one I am
>> trying to sell with this series for sensor devices. For instance, in a
>> sensor node I am attaching a phandle to describe how thermal fw must
>> behave. Then the sensor driver it is supposed to load the thermal data
>> into the thermal fw. Same could apply for instance for cpufreq cooling
>> device. at the cpu node we could have a 'cooling_device' node at the cpu
>> node, while loading cpufreq-cpu0.
> 
> I think a separate cooling_device node may be only necessary if we stuff
> additional info in there. If it's just a plain cooling device I think it
> is reasonable for the cpufreq driver to just register a cooling device
> if the thermal framework is there.

no, I think this is not what we want, because, as I said, not all cpus
will need cooling. Just because the thermal framework is there does not
mean your cpu needs cooling. As you can see, the thermal framework is
not only for cpu cooling. It can be used for any other thermal need.
Besides one needs to cover for the case where you are building for
multiple platform support. Assuming system needs based on Kconfig setup
is not very likely to scale in this case.

> 
> I would really like the information about a thermal zone to hang off one
> dt node rather than being scattered over several nodes. This way it may

Again, thermal framework is not about only cpu(freq) cooling. Thermal
zone info can (and will) be hanged off in one dt node. But please don't
mix concepts. Just because a cooling device is part of a thermal zone,
it does not mean it is only used there and that it can be defined there.
One can use a cooling device in different thermal zones.

> be easy to reference a cooling device in different thermal zones with
> different weight, etc. As long as we define a thermal zone to always be
> defined by a single sensor the right place seems to be the proposed
> subnode to the sensor. If we want a zone to have more than one sensor,
> we might even want a separate dt node for the thermal zone, referencing
> both sensors and cooling devices through phandles.

I still don't get why and how defining a thermal zone inside a sensor
phandle can prevent us defining a cooling device in different device
phandle.

> 
> Regards,
> Lucas
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]