Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1722214 > unrolled thread

[PATCH v4 3/3] arm64: dts: register Hi3660's thermal sensor

Started byTao Wang <kevin.wangtao@hisilicon.com>
First post2017-08-29 10:30 +0200
Last post2017-09-04 10:20 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v4 3/3] arm64: dts: register Hi3660's thermal sensor Tao Wang <kevin.wangtao@hisilicon.com> - 2017-08-29 10:30 +0200
    Re: [PATCH v4 3/3] arm64: dts: register Hi3660's thermal sensor Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-31 23:20 +0200
      Re: [PATCH v4 3/3] arm64: dts: register Hi3660's thermal sensor "Wangtao (Kevin, Kirin)" <kevin.wangtao@hisilicon.com> - 2017-09-04 10:20 +0200

#1722214 — [PATCH v4 3/3] arm64: dts: register Hi3660's thermal sensor

FromTao Wang <kevin.wangtao@hisilicon.com>
Date2017-08-29 10:30 +0200
Subject[PATCH v4 3/3] arm64: dts: register Hi3660's thermal sensor
Message-ID<ujK4h-f9-1@gated-at.bofh.it>
From: Tao Wang <kevin.wangtao@linaro.org>

Bind thermal sensor driver for Hi3660.

Signed-off-by: Tao Wang <kevin.wangtao@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.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@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_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
+#define MAX_TEMP		3
+#define AVG_TEMP		4
+
+#define HISI_MAX_TSENSORS		3
+#define THERMAL_NO_LIMIT		(~0)
+
+#endif
-- 
2.8.1

[toc] | [next] | [standalone]


#1724590

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2017-08-31 23:20 +0200
Message-ID<ukF2y-2z7-25@gated-at.bofh.it>
In reply to#1722214
On 29/08/2017 10:17, Tao Wang wrote:
> From: Tao Wang <kevin.wangtao@linaro.org>
> 
> Bind thermal sensor driver for Hi3660.

The description is a bit light :)


> Signed-off-by: Tao Wang <kevin.wangtao@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@linaro.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@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_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?

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.

> +#define HISI_MAX_TSENSORS		3> +#define THERMAL_NO_LIMIT		(~0)

Already defined in dt-bindings/thermal.h

> +#endif



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[toc] | [prev] | [next] | [standalone]


#1725919

From"Wangtao (Kevin, Kirin)" <kevin.wangtao@hisilicon.com>
Date2017-09-04 10:20 +0200
Message-ID<ulULU-35P-3@gated-at.bofh.it>
In reply to#1724590

在 2017/9/1 5:13, Daniel Lezcano 写道:
> On 29/08/2017 10:17, Tao Wang wrote:
>> From: Tao Wang <kevin.wangtao@linaro.org>
>>
>> Bind thermal sensor driver for Hi3660.
> 
> The description is a bit light :)
All right
> 
> 
>> Signed-off-by: Tao Wang <kevin.wangtao@linaro.org>
>> Signed-off-by: Leo Yan <leo.yan@linaro.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@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_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
> 
> 
> 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web