linux-kernel - Re: [PATCH v4 3/3] arm64: dts: register Hi3660's thermal sensor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 4 Sep 2017 16:11:12 +0800
From:   "Wangtao (Kevin, Kirin)" <kevin.wangtao@...ilicon.com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>, <rui.zhang@...el.com>,
        <edubezval@...il.com>, <robh+dt@...nel.org>,
        <mark.rutland@....com>, <xuwei5@...ilicon.com>,
        <catalin.marinas@....com>, <will.deacon@....com>
CC:     <leo.yan@...aro.org>, <kevin.wangtao@...aro.org>,
        <linux-pm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <sunzhaosheng@...ilicon.com>, <gengyanping@...ilicon.com>
Subject: Re: [PATCH v4 3/3] arm64: dts: register Hi3660's thermal sensor



在 2017/9/1 5:13, Daniel Lezcano 写道:
> On 29/08/2017 10:17, Tao Wang wrote:
>> From: Tao Wang <kevin.wangtao@...aro.org>
>>
>> Bind thermal sensor driver for Hi3660.
> 
> The description is a bit light :)
All right
> 
> 
>> Signed-off-by: Tao Wang <kevin.wangtao@...aro.org>
>> Signed-off-by: Leo Yan <leo.yan@...aro.org>
>> ---
>>   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 <dt-bindings/interrupt-controller/arm-gic.h>
>>   #include <dt-bindings/clock/hi3660-clock.h>
>> +#include <dt-bindings/thermal/hi3660-thermal.h>
>>   
>>   / {
>>   	compatible = "hisilicon,hi3660";
>> @@ -848,5 +849,18 @@
>>   				     &sdio_cfg_func>;
>>   			status = "disabled";
>>   		};
>> +
>> +		tsensor: tsensor@...30000 {
>> +			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_MAX_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 <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#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
> 
> 
>