Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753375AbdIDIOw (ORCPT ); Mon, 4 Sep 2017 04:14:52 -0400 Received: from szxga04-in.huawei.com ([45.249.212.190]:5517 "EHLO szxga04-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751949AbdIDIOu (ORCPT ); Mon, 4 Sep 2017 04:14:50 -0400 Subject: Re: [PATCH v4 3/3] arm64: dts: register Hi3660's thermal sensor To: Daniel Lezcano , , , , , , , CC: , , , , , , , References: <1503994666-13954-1-git-send-email-kevin.wangtao@hisilicon.com> <1503994666-13954-4-git-send-email-kevin.wangtao@hisilicon.com> <4e819928-9544-e7a1-b8ff-09ebee07f32b@linaro.org> From: "Wangtao (Kevin, Kirin)" Message-ID: Date: Mon, 4 Sep 2017 16:11:12 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <4e819928-9544-e7a1-b8ff-09ebee07f32b@linaro.org> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.121.88.70] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090201.59AD0AAB.004F,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 753f560601d5e7a40f134fe3a5d83157 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3619 Lines: 109 在 2017/9/1 5:13, Daniel Lezcano 写道: > On 29/08/2017 10:17, Tao Wang wrote: >> From: Tao Wang >> >> Bind thermal sensor driver for Hi3660. > > The description is a bit light :) All right > > >> Signed-off-by: Tao Wang >> Signed-off-by: Leo Yan >> --- >> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 14 +++++++++++++ >> include/dt-bindings/thermal/hi3660-thermal.h | 31 ++++++++++++++++++++++++++++ >> 2 files changed, 45 insertions(+) >> create mode 100644 include/dt-bindings/thermal/hi3660-thermal.h >> >> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi >> index c6a1961..1ee6084 100644 >> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi >> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi >> @@ -6,6 +6,7 @@ >> >> #include >> #include >> +#include >> >> / { >> compatible = "hisilicon,hi3660"; >> @@ -848,5 +849,18 @@ >> &sdio_cfg_func>; >> status = "disabled"; >> }; >> + >> + tsensor: tsensor@fff30000 { >> + compatible = "hisilicon,hi3660-tsensor"; >> + #address-cells = <2>; >> + #size-cells = <2>; >> + reg = <0x0 0xfff3001c 0x0 0x4>, >> + <0x0 0xfff3005c 0x0 0x4>, >> + <0x0 0xfff3009c 0x0 0x4>; >> + hisi,tsensors = ; >> + hisi,coef = <165000 (-40000)>; >> + hisi,adc-range = <0x74 0x39A>; >> + #thermal-sensor-cells = <1>; >> + }; >> }; >> }; >> diff --git a/include/dt-bindings/thermal/hi3660-thermal.h b/include/dt-bindings/thermal/hi3660-thermal.h >> new file mode 100644 >> index 0000000..d9d9b6a >> --- /dev/null >> +++ b/include/dt-bindings/thermal/hi3660-thermal.h >> @@ -0,0 +1,31 @@ >> +/* >> + * Copyright (c) 2017 Hisilicon Limited. >> + * Copyright (c) 2017 Linaro Limited. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; version 2 of the License. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see . >> + */ >> + >> +#ifndef __DTS_HI3660_THERMAL_H >> +#define __DTS_HI3660_THERMAL_H >> + >> +/* sensor id */ >> +#define LITTLE_CPU_TEMP 0 >> +#define BIG_CPU_TEMP 1 >> +#define GPU_TEMP 2 > > Is it possible to stick to the sensor description? OK > > eg. > > #define HI3660_A53_TEMP 0 > #define HI3660_A72_TEMP 1 > #define HI3660_GPU_TEMP 2 > >> +#define MAX_TEMP 3 >> +#define AVG_TEMP 4 > > It does not make sense to define AVG and MAX, they describe how is > computed the temperature. It makes more sense to define the group eg. > SOC_TEMP and add an attribute to the virtual sensor telling how you want > the temperature to be computed. That will make the code in the driver > much more simple, clear and clean. You inspired me, actually the thermal zone just want the SoC temp, my driver only need to offer one virtual sensor, we don't need the sensor id any more > >> +#define HISI_MAX_TSENSORS 3> +#define THERMAL_NO_LIMIT (~0) > > Already defined in dt-bindings/thermal.h > >> +#endif > > >