From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756505AbaIEJl0 (ORCPT ); Fri, 5 Sep 2014 05:41:26 -0400 Received: from hqemgate14.nvidia.com ([216.228.121.143]:3135 "EHLO hqemgate14.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755909AbaIEJlY (ORCPT ); Fri, 5 Sep 2014 05:41:24 -0400 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Fri, 05 Sep 2014 02:40:20 -0700 Message-ID: <54098538.1000505@nvidia.com> Date: Fri, 5 Sep 2014 17:41:12 +0800 From: Wei Ni User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Eduardo Valentin CC: Jean Delvare , Guenter Roeck , Stephen Warren , , "linux-tegra@vger.kernel.org" , LKML Subject: Re: [PATCH v3 3/4] thermal: add more description for thermal-zones References: <1408948188-4181-1-git-send-email-wni@nvidia.com> <1408948188-4181-4-git-send-email-wni@nvidia.com> <20140825110711.GA12349@developer> <53FBEE39.2020001@nvidia.com> <20140826121210.GB15219@developer> <53FD484F.7030504@nvidia.com> <20140827133217.GB15875@developer> <53FE8AC9.2070907@nvidia.com> <20140828132135.GA17074@developer> <53FFED7B.6010508@nvidia.com> <540449BC.2050807@nvidia.com> In-Reply-To: <540449BC.2050807@nvidia.com> X-Originating-IP: [10.19.224.131] X-ClientProxiedBy: DRBGMAIL103.nvidia.com (10.18.16.22) To HKMAIL101.nvidia.com (10.18.16.10) Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Eduardo On 09/01/2014 06:26 PM, Wei Ni wrote: > On 08/29/2014 07:32 PM, edubezval@gmail.com wrote: >> Hello Wei, >> >> On Thu, Aug 28, 2014 at 11:03 PM, Wei Ni wrote: >>> On 08/28/2014 09:21 PM, Eduardo Valentin wrote: >>>> Hello Wei, >>>> >>>> On Thu, Aug 28, 2014 at 09:50:01AM +0800, Wei Ni wrote: >>>>> On 08/27/2014 09:32 PM, Eduardo Valentin wrote: >>>>>> Hello Wei, >>>>>> >>>>>> On Wed, Aug 27, 2014 at 10:54:07AM +0800, Wei Ni wrote: >>>>>>> On 08/26/2014 08:12 PM, Eduardo Valentin wrote: >>>>>>>> On Tue, Aug 26, 2014 at 10:17:29AM +0800, Wei Ni wrote: >>>>>>>>> On 08/25/2014 07:07 PM, Eduardo Valentin wrote: >>>>>>>>>> Hello Wei Ni, >>>>>>>>>> >>>>>>>>>> On Mon, Aug 25, 2014 at 02:29:47PM +0800, Wei Ni wrote: >>>>>>>>>>> Add more description for the "polling-delay" property. >>>>>>>>>>> Set "trips" and "cooling maps" as optional property, because >>>>>>>>>>> if missing these two sub-nodes, the thermal zone device still >>>>>>>>>>> work properly. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Wei Ni >>>>>>>>>>> --- >>>>>>>>>>> Documentation/devicetree/bindings/thermal/thermal.txt | 10 ++++++---- >>>>>>>>>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>>>>>>>>> >>>>>>>>>>> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt >>>>>>>>>>> index f5db6b7..e3d3ed9 100644 >>>>>>>>>>> --- a/Documentation/devicetree/bindings/thermal/thermal.txt >>>>>>>>>>> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt >>>>>>>>>>> @@ -136,8 +136,8 @@ containing trip nodes and one sub-node containing all the zone cooling maps. >>>>>>>>>>> >>>>>>>>>>> Required properties: >>>>>>>>>>> - polling-delay: The maximum number of milliseconds to wait between polls >>>>>>>>>>> - Type: unsigned when checking this thermal zone. >>>>>>>>>>> - Size: one cell >>>>>>>>>>> + Type: unsigned when checking this thermal zone. If this value is 0, the >>>>>>>>>>> + Size: one cell driver will not run polling queue, but just cancel it. >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> The description above is specific to Linux kernel implementation >>>>>>>>>> nomenclature. DT description needs to be OS agnostic. >>>>>>>>>> >>>>>>>>>>> - polling-delay-passive: The maximum number of milliseconds to wait >>>>>>>>>>> Type: unsigned between polls when performing passive cooling. >>>>>>>>>>> @@ -148,14 +148,16 @@ Required properties: >>>>>>>>>>> phandles + sensor >>>>>>>>>>> specifier >>>>>>>>>>> >>>>>>>>>>> +Optional property: >>>>>>>>>>> - trips: A sub-node which is a container of only trip point nodes >>>>>>>>>>> Type: sub-node required to describe the thermal zone. >>>>>>>>>>> >>>>>>>>>>> - cooling-maps: A sub-node which is a container of only cooling device >>>>>>>>>>> Type: sub-node map nodes, used to describe the relation between trips >>>>>>>>>>> - and cooling devices. >>>>>>>>>>> + and cooling devices. If missing the "trips" property, >>>>>>>>>>> + This sub-node will not be parsed, because no trips can >>>>>>>>>>> + be bound to cooling devices. >>>>>>>>>> >>>>>>>>>> Do you mean if the thermal zone misses the "trips" property? Actually, >>>>>>>>>> the binding describes both, cooling-maps and trips, as required >>>>>>>>>> properties. Thus, both needs to be in place to consider the thermal zone >>>>>>>>>> as a proper described zone. >>>>>>>>> >>>>>>>>> I moved the "trips" and "cooling-maps" to optional property, because if >>>>>>>>> missing these two properties, the thermal zone devices still can be >>>>>>>>> registered, and the driver can work properly, it has the basic function, >>>>>>>>> can read temperature from thermal sysfs, although it doesn't have trips >>>>>>>>> and bind with cooling devices. >>>>>>>> >>>>>>>> >>>>>>>> If a thermal zone is used only for monitoring, then I believe it lost >>>>>>>> its purpose. As Maybe a different framework shall be used, such as hwmon, >>>>>>>> for instance? >>>>>>> >>>>>>> Yes, if we only use it for monitoring, we can use hwmon. But we have >>>>>>> more functions base on these two thermal zone devices. We have a >>>>>>> skin-temperature driver, which used nct1008's remote and local >>>>>>> temperatures to estimator the skin temperature. As you know the thermal >>>>>>> framework is more powerful, the remote/local sensors can be register as >>>>>>> thermal zone, then the skin-temp driver can use thermal_zone_get_temp() >>>>>>> to read their temperatures and then estimator skin's temp. We also will >>>>>>> set trips and cooling devices for this skin-temp. >>>>>> >>>>>> In this case, don't you think it would make sense to create a thermal >>>>>> zone for the skin temperature and add the required sensors (including >>>>>> the nct1008) in it? >>>>> >>>>> Hi, Eduardo >>>>> Yes, we will create a thermal zone for the skin-temp driver and add the >>>>> required sensors in it, but in here we want to register these required >>>>> sensors as thermal zone devices, then the thermal framework can export >>>>> functions to read these sensors temperature. If use hwmon framework, it >>>>> can't export nct1008's internal sensors, such as remote/local sensors, >>>>> and no exported functions to read these temperatures. If we doesn't use >>>>> the thermal/of-thermal framework, we have to develop a new framework to >>>>> parse those sensor nodes, and I think this new one may only have slight >>>>> difference with current thermal framework. >>>>> If we set those two properties as optional property, then we can use it >>>>> in this case simply. >>>>> >>>>> The skin-temp nodes will something like this: >>>>> >>>>> skin_temp: therm-est { >>>>> compatible = "nvidia,tegra124-therm-est"; >>>>> >>>>> #thermal-sensor-cells = <0>; >>>>> >>>>> sub-devs { >>>>> dev@0 { >>>>> /* we need to register nct1008_remote as thermal zone devices, so that >>>>> it's easy to handle it by using thermal framework's exported functions. */ >>>>> dev = <&nct1008_remote>; >>>>> }; >>>>> dev@1 { >>>>> dev = <&nct1008_local>; >>>>> }; >>>>> }; >>>>> }; >>>>> >>>>> thermal-zones { >>>>> skin-therm { >>>>> polling-delay-passive = <15000> >>>>> polling-delay = <0>; >>>>> >>>>> thermal-sensors = <&skin_temp>; >>>> >>>> The binding allows you to add several sensors in one thermal zone. >>>> Please have a look on the example (c) of the thermal.txt binding >>>> description. It might be that what you want to do is actually: >>>> thermal-sensors = <&ntc1008_local>, >>>> <&ntc1008_remote>; >>> >>> >>> Yes, we have considered it, but in current kernel, it only supports 1 >>> sensor per zone. and according to the example (c), it's supposed to only >>> support simple algorithm, such as: >>> /* hotspot = 100 * bandgap - 120 * adc + 484 */ >>> coefficients = <100 -120 484>; >>> >>> but in our skin-temp driver, we will recored 20 history temperatures for >>> every sensors, and set different coefficients for every history >>> temperatures, our HW team have private mathematical mode to calculate >>> these coeffs for different boards. And I think other users may use >>> different algorithm to calculate thermal zone temperature based on >>> several sensors. >>> >>> So I think it's better to register sensors to thermal zones, not set >>> trips and binding cooling devices, then user's thermal driver can use >>> them freely. >>> >> >> In your system, the used governor is userspace, then? > > No, we developed a new governor named as "pid governor", it manages > thermals based on output values of PID controller. > > Indeed, our thermal management includes skin-temp, soc-thermal, > cpu-throttle driver and pid governor. This pach set is prepared for the > skin-temp driver. > As you know the tegra soc-therm is in upstreaming, and we will post > other drivers step by step. Will you take this patch, if so i will post skin-temp driver later. Looking forward your comments :) Thanks. Wei. > > Thanks. > Wei. > >> >>> Thanks. >>> Wei. >>> >>>> >>>> >>>>> >>>>> trips { >>>>> }; >>>>> cooling-maps { >>>>> }; >>>>> }; >>>>> }; >>>>> >>>>> Wei. >>>>>> >>>>>>> >>>>>>> Wei. >>>>>>> >>>>>>>> >>>>>>>> The purpose of a thermal zone is to describe thermal behavior of a >>>>>>>> hardware. As it is mentioned in the thermal.txt file. >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> Thanks. >>>>>>>>> Wei. >>>>>>>>> >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -Optional property: >>>>>>>>>>> - coefficients: An array of integers (one signed cell) containing >>>>>>>>>>> Type: array coefficients to compose a linear relation between >>>>>>>>>>> Elem size: one cell the sensors listed in the thermal-sensors property. >>>>>>>>>>> -- >>>>>>>>>>> 1.8.1.5 >>>>>>>>>>> >>>>>>>>> >>>>>>>> -- >>>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in >>>>>>>> the body of a message to majordomo@vger.kernel.org >>>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>>>>> >>>>>>> >>>>> >>> >> >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-tegra" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >