Re: [PATCH 1/2] thermal: db8500: Finalize device tree conversion

From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Zhang Rui <rui.zhang@intel.com>,
	Eduardo Valentin <edubezval@gmail.com>,
	Linux PM list <linux-pm@vger.kernel.org>,
	Lee Jones <lee.jones@linaro.org>
Subject: Re: [PATCH 1/2] thermal: db8500: Finalize device tree conversion
Date: Tue, 27 Aug 2019 15:24:49 +0200	[thread overview]
Message-ID: <c4b0e2a8-90b9-2e76-e8ad-7e28fc762389@linaro.org> (raw)
In-Reply-To: <CACRpkdYqjW_SHHLaSqgWO64CsGeyoOVGsOhnav0mwVA5W2fbuw@mail.gmail.com>

Hi Linus,

On 27/08/2019 13:21, Linus Walleij wrote:
> On Thu, Aug 22, 2019 at 8:16 AM Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
> 
>> I've been through the DT and I don't understand why there is:
>>
>>         [ ... ]
>>         trip0-temp = <70000>;
>>         trip0-type = "active";
>>         trip0-cdev-num = <1>;
>>         trip0-cdev-name0 = "thermal-cpufreq-0";
>>         [ ... ]
>>
>> Those bindings already exists for the thermal no?
>>
>> Why not create a thermal-zone node and then add the values above?
> 
> I guess you refer to arch/arm/boot/dts/ste-snowball.dts
> where this is in the thermal@801573c0 node.
> (I moved it to the main SoC file for the next kernel.)
> 
> I think the reason is that this was merged in:
> commit dc1956b5f84de7d453ec4d9fe68385fffd689686
> "Thermal: Add ST-Ericsson DB8500 thermal properties and platform data."
> Date:   Thu Nov 15 18:56:43 2012 +0800
> 
> Thermal zones were not added until:
> commit 4e5e4705bf69ea450f58fc709ac5888f321a9299
> "thermal: introduce device tree parser"
> Date:   Wed Jul 3 15:35:39 2013 -0400

Oh, right. Time is running away ... :)

> Which is half a year after this device tree was written.
> 
> Apparently Hongbo stopped working on this driver before
> that and didn't convert the driver or consumers over to the
> new thermal zone APIs with trip points and cooling maps
> defined in the device tree (which by the way is an awesome
> feature).
> 
>> Another point is there are too many trip points, two should be enough,
>> one for throttling and one for critical, the governor will handle that
>> properly by stepping the opps.
>>
>> And one last point is the trip point should be passive, not active.
> 
> OK I get it, can we merge these two patches that just move
> the code to the thermal driver and then I can make
> a new patch or some new patches on top to migrate to
> Eduardo's new framework for device tree defined thermal
> zones?

Sounds good to me

-- 
 <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