From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754340AbcAOHUa (ORCPT ); Fri, 15 Jan 2016 02:20:30 -0500 Received: from hqemgate16.nvidia.com ([216.228.121.65]:11662 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753261AbcAOHU2 convert rfc822-to-8bit (ORCPT ); Fri, 15 Jan 2016 02:20:28 -0500 X-PGP-Universal: processed; by hqnvupgp07.nvidia.com on Thu, 14 Jan 2016 23:22:22 -0800 Message-ID: <56989DDF.2060004@nvidia.com> Date: Fri, 15 Jan 2016 15:21:03 +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: Thierry Reding CC: , , , , , Subject: Re: [PATCH V1 06/10] of: Add bindings of hw-trips for soctherm References: <1452672022-387-1-git-send-email-wni@nvidia.com> <20160113152527.GF2588@ulmo> In-Reply-To: <20160113152527.GF2588@ulmo> 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 On 2016年01月13日 23:25, Thierry Reding wrote: > * PGP Signed by an unknown key > > Cc'ing devicetree@vger.kernel.org, please make sure to always Cc that > list on device tree binding patches. > > On Wed, Jan 13, 2016 at 04:00:22PM +0800, Wei Ni wrote: >> Add hw-trips sub-node for soctherm, which is >> used to describe the hardware trip points for >> each soctherm sensors. > > Perhaps you should mention Tegra in the subject and the commit message? > > Also please use all of the 72 columns that are at your disposal when > writing commit messages. > >> Signed-off-by: Wei Ni >> --- >> .../devicetree/bindings/thermal/tegra-soctherm.txt | 29 ++++++++++++++++++++++ >> 1 file changed, 29 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/thermal/tegra-soctherm.txt b/Documentation/devicetree/bindings/thermal/tegra-soctherm.txt >> index 6b68cd150405..77aef0b4968b 100644 >> --- a/Documentation/devicetree/bindings/thermal/tegra-soctherm.txt >> +++ b/Documentation/devicetree/bindings/thermal/tegra-soctherm.txt >> @@ -26,6 +26,18 @@ Required properties : >> of this property. See for a >> list of valid values when referring to thermal sensors. >> >> +Optional properties: >> +- hw-trips : A sub-node which is a container of hardware trip points >> + for each sensors. > > Perhaps call the node "trip-points"? "hw-" seems kind of redundant since > DT is a hardware description, and "trips" isn't a very good abbreviation > for "trip-points". In the linux thermal framework, it has the node definition "trips" and have sysfs interface named as "trip-points", these "trip" are set for SW. If we also use the "trip-points" in here, it will make confusion. I used "hw-", because it's different with the SW trip-points, the trip-points under it are for the Tegra to shutdown/reset from HW, or trigger the HW-throttle. (not SW throttle, see below) How about "hw-trip-points"? > >> + * sensors: Sub-nodes which are used to describe the HW trip points info, >> + must be named as "cpu", "gpu", "mem", "pll". >> + Properties: >> + - therm-temp : Shutdown temperature in millicelsius, once the >> + temperature of this sensor is higher than the therm-temp, the system >> + will be reset or shutdown. > > Why not call the property "shutdown-temperature" if that's what it is. This is the shutdown/reset temperature. The Tegra's SPEC calls it as thermtrip event, when the temperature touch this value, it will trigger the thermtrip event, and then will cause the system reset or shutdown, it depends on the HW platform design. So I named it as "therm-temp". I think it's better to change to "thermtrip-temperature". > >> + - throt-temp : HW throttle temperature in millicelsius, once the >> + temperature of this sensor is higher than the throt-temp, the HW >> + throttle will be triggered in soctherm. (Not implement yet) > > "throttle-temperature" would be a better name in my opinion. More typing > perhaps but less chance of getting the abbreviations wrong. Also there's > no need for the "(Not implement yet)" because the binding isn't about an > implementation, it's about the hardware description. If you have no code > that uses this, a safer option might be to just drop it for now. The Tegra soctherm support throttle function, when the temperature touch the trip point, the HW will skip the CPU/GPU' pulse, so I called it as HW throttle. For the SW throttle, the SW will cap the CPU/GPU or other clocks frequencies. Anyway, since we didn't support it yet, I will drop it for now. > > Thierry > > * Unknown Key > * 0x7F3EB3A1 >