Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752871AbcD2Fxk (ORCPT ); Fri, 29 Apr 2016 01:53:40 -0400 Received: from nat-hk.nvidia.com ([203.18.50.4]:62539 "EHLO hkmmgate102.nvidia.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751837AbcD2Fxi convert rfc822-to-8bit (ORCPT ); Fri, 29 Apr 2016 01:53:38 -0400 X-PGP-Universal: processed; by hkpgpgate101.nvidia.com on Thu, 28 Apr 2016 22:53:34 -0700 Message-ID: <5722F73A.7020606@nvidia.com> Date: Fri, 29 Apr 2016 13:55:06 +0800 From: Wei Ni User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Eduardo Valentin CC: , , , , , , , , , Subject: Re: [PATCH v2 4/9] of: Add bindings of hw throttle for soctherm References: <1461727554-15065-1-git-send-email-wni@nvidia.com> <1461727554-15065-5-git-send-email-wni@nvidia.com> <20160427233054.GA26451@localhost.localdomain> <5721B249.3010101@nvidia.com> <20160428144828.GB19279@localhost.localdomain> In-Reply-To: <20160428144828.GB19279@localhost.localdomain> X-Originating-IP: [10.19.224.146] X-ClientProxiedBy: HKMAIL101.nvidia.com (10.18.16.10) To HKMAIL101.nvidia.com (10.18.16.10) Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8373 Lines: 190 On 2016年04月28日 22:48, Eduardo Valentin wrote: > On Thu, Apr 28, 2016 at 02:48:41PM +0800, Wei Ni wrote: >> >> >> On 2016年04月28日 07:30, Eduardo Valentin wrote: >>> From: Eduardo Valentin >>> To: Wei Ni >>> Cc: thierry.reding@gmail.com, robh+dt@kernel.org, rui.zhang@intel.com, >>> MLongnecker@nvidia.com, swarren@wwwdotorg.org, >>> mikko.perttunen@kapsi.fi, linux-tegra@vger.kernel.org, >>> linux-pm@vger.kernel.org, devicetree@vger.kernel.org, >>> linux-kernel@vger.kernel.org >>> Bcc: >>> Subject: Re: [PATCH v2 4/9] of: Add bindings of hw throttle for soctherm >>> Reply-To: >>> In-Reply-To: <1461727554-15065-5-git-send-email-wni@nvidia.com> >>> >>> The patch title must say something about the fact that this is specific >>> to nvidia thermal driver. >> >> Yes, it's my mistake, will fix it in next series. >> >>> >>> On Wed, Apr 27, 2016 at 11:25:49AM +0800, Wei Ni wrote: >>>> Add HW throttle configuration sub-node for soctherm, which >>>> is used to describe the throttle event, and worked as a >>>> cooling device. The "hot" type trip in thermal zone can >>>> be bound to this cooling device, and trigger the throttle >>>> function. >>>> >>>> Signed-off-by: Wei Ni >>>> --- >>>> .../bindings/thermal/nvidia,tegra124-soctherm.txt | 89 +++++++++++++++++++++- >>>> 1 file changed, 87 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.txt b/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.txt >>>> index edebfa0a985e..dc337d139f49 100644 >>>> --- a/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.txt >>>> +++ b/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.txt >>>> @@ -10,8 +10,14 @@ Required properties : >>>> - compatible : For Tegra124, must contain "nvidia,tegra124-soctherm". >>>> For Tegra132, must contain "nvidia,tegra132-soctherm". >>>> For Tegra210, must contain "nvidia,tegra210-soctherm". >>>> -- reg : Should contain 1 entry: >>>> +- reg : Should contain at least 2 entries for each entry in reg-names: >>>> - SOCTHERM register set >>>> + - Tegra CAR register set: Required for Tegra124 and Tegra210. >>>> + - CCROC register set: Required for Tegra132. >>>> +- reg-names : Should contain at least 2 entries: >>>> + - soctherm-reg >>>> + - car-reg >>>> + - ccroc-reg >>>> - interrupts : Defines the interrupt used by SOCTHERM >>>> - clocks : Must contain an entry for each entry in clock-names. >>>> See ../clocks/clock-bindings.txt for details. >>>> @@ -25,17 +31,44 @@ Required properties : >>>> - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description >>>> of this property. See for a >>>> list of valid values when referring to thermal sensors. >>>> +- throttle-cfgs: A sub-node which is a container of configuration for each >>>> + hardware throttle events. These events can be set as cooling devices. >>>> + * throttle events: Sub-nodes must be named as "light" or "heavy". >>>> + Properties: >>>> + - priority: Each throttles has its own throttle settings, so the SW need >>>> + to set priorities for various throttle, the HW arbiter can select the >>>> + final throttle settings. >>>> + Bigger value indicates higher priority, In general, higher priority >>>> + translates to lower target frequency. SW needs to ensure that critical >>>> + thermal alarms are given higher priority, and ensure that there is >>>> + no race if priority of two vectors is set to the same value. >>>> + - cpu-throt-depth: This property is for Tegra124 and Tegra210. It is >>>> + the throttling depth of pulse skippers, it's the percentage >>>> + throttling. >>>> + - cpu-throt-level: This property is only for Tegra132, it is the level >>>> + of pulse skippers, which used to throttle clock frequencies. It >>>> + indicates cpu clock throttling depth, and the depth can be programmed. >>>> + Must set as following values: >>>> + TEGRA_SOCTHERM_THROT_LEVEL_LOW, TEGRA_SOCTHERM_THROT_LEVEL_MED >>>> + TEGRA_SOCTHERM_THROT_LEVEL_HIGH, TEGRA_SOCTHERM_THROT_LEVEL_NONE >>> >>> These properties are not generic properties. My understanding is that >>> you must have vendor prefix in such case. Same applies to the new nodes. >> >> Ok, will do it. >> >>> >>>> + - #cooling-cells: Should be 1. This cooling device only support on/off state. >>>> + See ./thermal.txt for a description of this property. >>>> >>>> Note: >>>> - the "critical" type trip points will be set to SOC_THERM hardware as the >>>> shut down temperature. Once the temperature of this thermal zone is higher >>>> than it, the system will be shutdown or reset by hardware. >>>> +- the "hot" type trip points will be set to SOC_THERM hardware as the throttle >>>> +temperature. Once the the temperature of this thermal zone is higher >>>> +than it, it will trigger the HW throttle event. >>>> >>>> Example : >>>> >>>> soctherm@700e2000 { >>>> compatible = "nvidia,tegra124-soctherm"; >>>> - reg = <0x0 0x700e2000 0x0 0x1000>; >>>> + reg = <0x0 0x700e2000 0x0 0x600 /* SOC_THERM reg_base */ >>>> + 0x0 0x60006000 0x0 0x400 /* CAR reg_base */ >>>> + reg-names = "soctherm-reg", "car-reg"; >>>> interrupts = ; >>>> clocks = <&tegra_car TEGRA124_CLK_TSENSOR>, >>>> <&tegra_car TEGRA124_CLK_SOC_THERM>; >>>> @@ -44,6 +77,45 @@ Example : >>>> reset-names = "soctherm"; >>>> >>>> #thermal-sensor-cells = <1>; >>>> + >>>> + throttle-cfgs { >>>> + throttle_heavy: heavy { >>>> + priority = <100>; >>>> + cpu-throt-depth = <85>; >>>> + >>>> + #cooling-cells = <1>; >>>> + }; >>>> + throttle_light: light { >>>> + priority = <80>; >>>> + cpu-throt-depth = <50>; >>>> + >>>> + #cooling-cells = <1>; >>>> + }; >>>> + }; >>> >>> This is a sensor, which at the same time, has sub nodes that can be >>> cooling devices. Is my understanding correct of what you are trying to >>> do? >> >> Yes, we set this sub nodes as cooling devices. >> Let me explain it. >> The Tegra's soctherm has the HW throttle function, it will be triggered when the >> temperatures reach to the "hot" type trip. These throttle events can be >> configured to different levels "heavy" and "light" on different sensors. So it >> something like cooling devices which can be bound with thermal zones, and in >> this way, we can use the linux of-thermal framework to handle it. If not, we >> have to add more codes to parse these configuration and set them for those >> sensors. I had used this method to handle the shutdown function in Tegra's >> soctherm driver in my previous series, however Rob didn't like it, he preferred >> to use linux framework to handle it. >> So I think it's better to set these throttle-cfgs as cooling devices, so they >> can be bound to the cpu/gpu thermal zones easily. >> >>> >>> >>>> + }; >>>> + >>>> +Example: referring to Tegra132's "reg", "reg-names" and "throttle-cfgs" : >>>> + >>>> + soctherm@0,700e2000 { >>>> + compatible = "nvidia,tegra132-soctherm"; >>>> + reg = <0x0 0x700e2000 0x0 0x600 /* SOC_THERM reg_base */ >>>> + 0x0 0x70040000 0x0 0x200>; /* CCROC reg_base */; >>>> + reg-names = "soctherm-reg", "ccroc-reg"; >>>> + >>>> + throttle-cfgs { >>>> + throttle_heavy: heavy { >>>> + priority = <100>; >>>> + cpu-throt-level = ; >>>> + >>>> + #cooling-cells = <1>; >>>> + }; >>>> + throttle_light: light { >>>> + priority = <80>; >>>> + cpu-throt-level = ; >>>> + >>>> + #cooling-cells = <1>; >>> >>> Could you please describe a little here what to expect of the above >>> setup? >> >> Ok, let me describe it. >> When the "heavy" cooling device triggered, the hardware will skip cpu clock's >> pulse in HIGH level, and if the "light" triggered, the hardware will skip cpu >> clock's pulse in MED level. >> If these two devices are triggered in same time, the HW throttle arbiter will >> select the highest priority as the final throttle settings to skip cpu pulse. > > > I would prefer if you add these descriptions into your examples in your > next version. Sure, will do it. >