All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/5] Rockchip soc thermal driver
@ 2014-10-10 10:19 ` Caesar Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Caesar Wang @ 2014-10-10 10:19 UTC (permalink / raw)
  To: heiko, rui.zhang, edubezval, arnd
  Cc: zyf, dianders, linux-rockchip, linux-kernel, linux-pm,
	linux-arm-kernel, devicetree, linux-doc, cf, dmitry.torokhov,
	dbasehore, huangtao, cjf, zhengsq, Caesar Wang

This series patchs tested on rk3288 SDK board and pinky-v1,v2 board.
I believe the driver can be used on the rk3288-evb board.

Add this driver, The system can reset the entire chip when
the thermal temperture over 120C, In case of rising over 125C
when tha hardware shorting,The sodftware will shutdown via "critical".

Changes in v8:
	* address comments from Dmitry Torokhov and Doug Anderson.
	- add three Teperture Sensors.
        - support CRU and GPIO reset chip.
	- rename rk3xxx-cpu-thermal.dtsi as rk3288-thermal.dtsi
	- alarm-temp via set_trips() callback
	- remove "reset-gpios"

Changes in v7:
        - fix get data->clk=0 when in probe() function.
	- fix some style code.
	- modify dts,main add rk3xxx-cpu-thermal.dtsi

Changes in v6:
        * address comments from Tomeu Vizoso.
        - use thermal's generic framework.

Changes in v5:
        * address comments from Eduardo Valentin,rui.zhang and Heiko Stubner:
        - with BIT() macro
        - manage clocks in suspend/resume.
	- license is fixed as GPLv2.
	- #include "thermal_core.h"->#include <linux/thermal.h>
	- use the generic trip-points.the hw-shut-temp isn't generic trip-points.
	- The method of binding and unbinding be fixed.
	- The pin-name tsadc->otp_out

Changes in v4:
        * address comments from Jonathan Cameron,huangtao and zhaoyifeng:
        - this series thermal driver still be put in driver/thermal/
        - modify the thermal driver description.

Changes in v3:(add dts configure)
        * address comments from Dmitry Torokhov and Arnd Bergmann:
        - fix clock-names in rockchip-thermal.txt
        - remove rockchip_thermal_control() in rockchip_set_mode()
        - fix some code style.
        - add dts configure.

Changes in v2:
        * address comments from Heiko Stubner:
        - fix dt-bindings in rockchip-thermal.txt
        - remove Author mark
        - rename TSADC_XXX->TSADCV2_XXX,it eill ready to merge compatible other SoCs.
        - fix a identation
        - remove clk_set_rate(),it's no necessary.
        - fix the SIMPLE_DEV_PM_OPS() function  style.

Caesar Wang (5):
  thermal: rockchip: add driver for thermal
  dt-bindings: document Rockchip thermal
  ARM: dts: add RK3288 Thermal data
  ARM: dts: add main Thermal info to rk3288
  ARM: dts: enable Thermal on rk3288-evb board

 .../bindings/thermal/rockchip-thermal.txt          |  45 ++
 arch/arm/boot/dts/rk3288-evb.dtsi                  |   5 +
 arch/arm/boot/dts/rk3288-thermal.dtsi              |  58 ++
 arch/arm/boot/dts/rk3288.dtsi                      |  23 +
 drivers/thermal/Kconfig                            |   9 +
 drivers/thermal/Makefile                           |   1 +
 drivers/thermal/rockchip_thermal.c                 | 628 +++++++++++++++++++++
 7 files changed, 769 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
 create mode 100644 arch/arm/boot/dts/rk3288-thermal.dtsi
 create mode 100644 drivers/thermal/rockchip_thermal.c

-- 
1.9.1



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v8 0/5] Rockchip soc thermal driver
@ 2014-10-10 10:19 ` Caesar Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Caesar Wang @ 2014-10-10 10:19 UTC (permalink / raw)
  To: linux-arm-kernel

This series patchs tested on rk3288 SDK board and pinky-v1,v2 board.
I believe the driver can be used on the rk3288-evb board.

Add this driver, The system can reset the entire chip when
the thermal temperture over 120C, In case of rising over 125C
when tha hardware shorting,The sodftware will shutdown via "critical".

Changes in v8:
	* address comments from Dmitry Torokhov and Doug Anderson.
	- add three Teperture Sensors.
        - support CRU and GPIO reset chip.
	- rename rk3xxx-cpu-thermal.dtsi as rk3288-thermal.dtsi
	- alarm-temp via set_trips() callback
	- remove "reset-gpios"

Changes in v7:
        - fix get data->clk=0 when in probe() function.
	- fix some style code.
	- modify dts,main add rk3xxx-cpu-thermal.dtsi

Changes in v6:
        * address comments from Tomeu Vizoso.
        - use thermal's generic framework.

Changes in v5:
        * address comments from Eduardo Valentin,rui.zhang and Heiko Stubner:
        - with BIT() macro
        - manage clocks in suspend/resume.
	- license is fixed as GPLv2.
	- #include "thermal_core.h"->#include <linux/thermal.h>
	- use the generic trip-points.the hw-shut-temp isn't generic trip-points.
	- The method of binding and unbinding be fixed.
	- The pin-name tsadc->otp_out

Changes in v4:
        * address comments from Jonathan Cameron,huangtao and zhaoyifeng:
        - this series thermal driver still be put in driver/thermal/
        - modify the thermal driver description.

Changes in v3:(add dts configure)
        * address comments from Dmitry Torokhov and Arnd Bergmann:
        - fix clock-names in rockchip-thermal.txt
        - remove rockchip_thermal_control() in rockchip_set_mode()
        - fix some code style.
        - add dts configure.

Changes in v2:
        * address comments from Heiko Stubner:
        - fix dt-bindings in rockchip-thermal.txt
        - remove Author mark
        - rename TSADC_XXX->TSADCV2_XXX,it eill ready to merge compatible other SoCs.
        - fix a identation
        - remove clk_set_rate(),it's no necessary.
        - fix the SIMPLE_DEV_PM_OPS() function  style.

Caesar Wang (5):
  thermal: rockchip: add driver for thermal
  dt-bindings: document Rockchip thermal
  ARM: dts: add RK3288 Thermal data
  ARM: dts: add main Thermal info to rk3288
  ARM: dts: enable Thermal on rk3288-evb board

 .../bindings/thermal/rockchip-thermal.txt          |  45 ++
 arch/arm/boot/dts/rk3288-evb.dtsi                  |   5 +
 arch/arm/boot/dts/rk3288-thermal.dtsi              |  58 ++
 arch/arm/boot/dts/rk3288.dtsi                      |  23 +
 drivers/thermal/Kconfig                            |   9 +
 drivers/thermal/Makefile                           |   1 +
 drivers/thermal/rockchip_thermal.c                 | 628 +++++++++++++++++++++
 7 files changed, 769 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
 create mode 100644 arch/arm/boot/dts/rk3288-thermal.dtsi
 create mode 100644 drivers/thermal/rockchip_thermal.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v8 1/5] thermal: rockchip: add driver for thermal
  2014-10-10 10:19 ` Caesar Wang
@ 2014-10-10 10:19   ` Caesar Wang
  -1 siblings, 0 replies; 14+ messages in thread
From: Caesar Wang @ 2014-10-10 10:19 UTC (permalink / raw)
  To: heiko, rui.zhang, edubezval, arnd
  Cc: zyf, dianders, linux-rockchip, linux-kernel, linux-pm,
	linux-arm-kernel, devicetree, linux-doc, cf, dmitry.torokhov,
	dbasehore, huangtao, cjf, zhengsq, Caesar Wang

Thermal is TS-ADC Controller module supports
user-defined mode and automatic mode.

User-defined mode refers,TSADC all the control signals entirely by
software writing to register for direct control.

Automaic mode refers to the module automatically poll TSADC output,
and the results were checked.If you find that the temperature High
in a period of time,an interrupt is generated to the processor
down-measures taken;If the temperature over a period of time High,
the resulting TSHUT gave CRU module,let it reset the entire chip,
or via GPIO give PMIC.

Signed-off-by: zhaoyifeng <zyf@rock-chips.com>
Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
---
 drivers/thermal/Kconfig            |   9 +
 drivers/thermal/Makefile           |   1 +
 drivers/thermal/rockchip_thermal.c | 628 +++++++++++++++++++++++++++++++++++++
 3 files changed, 638 insertions(+)
 create mode 100644 drivers/thermal/rockchip_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index f9a1386..24845ae 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -133,6 +133,15 @@ config SPEAR_THERMAL
 	  Enable this to plug the SPEAr thermal sensor driver into the Linux
 	  thermal framework.
 
+config ROCKCHIP_THERMAL
+	tristate "Rockchip thermal driver"
+	depends on ARCH_ROCKCHIP
+	help
+	  Rockchip thermal driver provides support for Temperature sensor
+	  ADC (TS-ADC) found on Rockchip SoCs. It supports one critical
+	  trip point. Cpufreq is used as the cooling device and will throttle
+	  CPUs when the Temperature crosses the passive trip point.
+
 config RCAR_THERMAL
 	tristate "Renesas R-Car thermal driver"
 	depends on ARCH_SHMOBILE || COMPILE_TEST
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index de0636a..930554f 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -19,6 +19,7 @@ thermal_sys-$(CONFIG_CPU_THERMAL)	+= cpu_cooling.o
 
 # platform thermal drivers
 obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
+obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
 obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
 obj-$(CONFIG_KIRKWOOD_THERMAL)  += kirkwood_thermal.o
 obj-y				+= samsung/
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
new file mode 100644
index 0000000..ceee2c1
--- /dev/null
+++ b/drivers/thermal/rockchip_thermal.c
@@ -0,0 +1,628 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ */
+
+#include <linux/clk.h>
+#include <linux/cpu_cooling.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+/**
+* If the temperature over a period of time High,
+* the resulting TSHUT gave CRU module,let it reset the entire chip,
+* or via GPIO give PMIC.
+*/
+enum reset_mde {
+	CRU = 0,
+	GPIO,
+};
+
+/**
+* The system has three Teperture Sensors.
+* channel0 is reserve,channel1 is for CPU,and
+* channel channel 2 is for GPU.
+*/
+enum sensor_id {
+	RESERVE = 0,
+	CPU,
+	GPU,
+	SENSOR_ID_END,
+};
+
+struct rockchip_thermal_data {
+	const struct rockchip_tsadc_platform_data *pdata;
+	struct thermal_zone_device *tz[SENSOR_ID_END];
+	struct thermal_cooling_device *cdev;
+	void __iomem *regs;
+
+	unsigned long temp_passive;
+	unsigned long hw_shut_temp;
+	unsigned long alarm_temp;
+	bool irq_enabled;
+	int irq;
+	int reset_mode;
+	int chn;
+
+	struct clk *clk;
+	struct clk *pclk;
+};
+
+struct rockchip_tsadc_platform_data {
+	unsigned long temp_passive;
+	unsigned long hw_shut_temp;
+	int reset_mode;
+
+	int (*irq_handle)(void __iomem *reg);
+	int (*initialize)(int reset_mode, int chn, void __iomem *reg,
+			  unsigned long hw_shut_temp);
+	int (*control)(void __iomem *reg, bool on);
+	int (*code_to_temp)(u32 code);
+	u32 (*temp_to_code)(int temp);
+	void (*set_alarm_temp)(int chn, void __iomem *reg,
+			       unsigned long alarm_temp);
+};
+
+/* TSADC V2 Sensor info define: */
+#define TSADCV2_AUTO_CON			0x04
+#define TSADCV2_INT_EN				0x08
+#define TSADCV2_INT_PD				0x0c
+#define TSADCV2_DATA(chn)			(0x20+chn*0x04)
+#define TSADCV2_COMP_INT(chn)		        (0x30+chn*0x04)
+#define TSADCV2_COMP_SHUT(chn)		        (0x40+chn*0x04)
+#define TSADCV2_HIGHT_INT_DEBOUNCE		0x60
+#define TSADCV2_HIGHT_TSHUT_DEBOUNCE		0x64
+#define TSADCV2_AUTO_PERIOD			0x68
+#define TSADCV2_AUTO_PERIOD_HT			0x6c
+
+#define TSADCV2_AUTO_EN				BIT(0)
+#define TSADCV2_AUTO_DISABLE			~BIT(0)
+#define TSADCV2_AUTO_SRC_EN(chn)	        (0xf << (4 + chn))
+#define TSADCV2_AUTO_TSHUT_POLARITY_HIGH	BIT(8)
+
+#define TSADCV2_INT_SRC_EN(chn)			BIT(chn)
+#define TSADCV2_SHUT_2GPIO_SRC_EN(chn)		(0xf << (4 + chn))
+#define TSADCV2_SHUT_2CRU_SRC_EN(chn)		(0xf << (8 + chn))
+
+#define TSADCV2_INT_PD_CLEAR			~BIT(8)
+
+#define TSADCV2_DATA_MASK			0xfff
+#define TSADCV2_HIGHT_INT_DEBOUNCE_TIME		0x0a
+#define TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME	0x0a
+#define TSADCV2_AUTO_PERIOD_TIME		0x03e8
+#define TSADCV2_AUTO_PERIOD_HT_TIME		0x64
+
+struct tsadc_table {
+	unsigned long code;
+	int temp;
+};
+
+static const struct tsadc_table v2_code_table[] = {
+	{TSADCV2_DATA_MASK, -40000},
+	{3800, -40000},
+	{3792, -35000},
+	{3783, -30000},
+	{3774, -25000},
+	{3765, -20000},
+	{3756, -15000},
+	{3747, -10000},
+	{3737, -5000},
+	{3728, 0},
+	{3718, 5000},
+	{3708, 10000},
+	{3698, 15000},
+	{3688, 20000},
+	{3678, 25000},
+	{3667, 30000},
+	{3656, 35000},
+	{3645, 40000},
+	{3634, 45000},
+	{3623, 50000},
+	{3611, 55000},
+	{3600, 60000},
+	{3588, 65000},
+	{3575, 70000},
+	{3563, 75000},
+	{3550, 80000},
+	{3537, 85000},
+	{3524, 90000},
+	{3510, 95000},
+	{3496, 100000},
+	{3482, 105000},
+	{3467, 110000},
+	{3452, 115000},
+	{3437, 120000},
+	{3421, 125000},
+	{0, 125000},
+};
+
+static int rk_tsadcv2_irq_handle(void __iomem *regs)
+{
+	u32 val;
+
+	val = readl_relaxed(regs + TSADCV2_INT_PD);
+	writel_relaxed(val & TSADCV2_INT_PD_CLEAR, regs + TSADCV2_INT_PD);
+
+	return 0;
+}
+
+static u32 rk_tsadcv2_temp_to_code(int temp)
+{
+	int high, low, mid, ret = 0;
+
+	low = 0;
+	high = ARRAY_SIZE(v2_code_table) - 1;
+	mid = (high + low) / 2;
+
+	if (temp < v2_code_table[low].temp || temp > v2_code_table[high].temp) {
+		ret = -ERANGE;
+		goto exit;
+	}
+
+	while (low <= high) {
+		if (temp == v2_code_table[mid].temp)
+			return v2_code_table[mid].code;
+		else if (temp < v2_code_table[mid].temp)
+			high = mid - 1;
+		else
+			low = mid + 1;
+		mid = (low + high) / 2;
+	}
+
+	return 0;
+
+exit:
+	return ret;
+}
+
+static int rk_tsadcv2_code_to_temp(u32 code)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(v2_code_table) - 1; i++) {
+		if (code >= v2_code_table[i].code)
+			return v2_code_table[i].temp;
+	}
+
+	/* No code available,return max temperture */
+	return 125000;
+}
+
+static int rk_tsadcv2_initialize(int reset_mode, int chn, void __iomem *regs,
+				 unsigned long hw_shut_temp)
+{
+	u32 shutdown_value;
+
+	shutdown_value = rk_tsadcv2_temp_to_code(hw_shut_temp);
+
+	/* Enable measurements at ~ 10 Hz */
+	writel_relaxed(0 | TSADCV2_AUTO_TSHUT_POLARITY_HIGH, regs +
+		       TSADCV2_AUTO_CON);
+	writel_relaxed(TSADCV2_AUTO_PERIOD_TIME, regs + TSADCV2_AUTO_PERIOD);
+	writel_relaxed(TSADCV2_AUTO_PERIOD_HT_TIME, regs +
+		       TSADCV2_AUTO_PERIOD_HT);
+	writel_relaxed(shutdown_value, regs + TSADCV2_COMP_SHUT(chn));
+	writel_relaxed(TSADCV2_HIGHT_INT_DEBOUNCE_TIME, regs +
+		       TSADCV2_HIGHT_INT_DEBOUNCE);
+	writel_relaxed(TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME, regs +
+		       TSADCV2_HIGHT_TSHUT_DEBOUNCE);
+
+	if (reset_mode == GPIO)
+		writel_relaxed(TSADCV2_SHUT_2GPIO_SRC_EN(chn) |
+			       TSADCV2_INT_SRC_EN(chn), regs +
+			       TSADCV2_INT_EN);
+	else
+		writel_relaxed(TSADCV2_SHUT_2CRU_SRC_EN(chn) |
+			       TSADCV2_INT_SRC_EN(chn) , regs +
+			       TSADCV2_INT_EN);
+
+	writel_relaxed(TSADCV2_AUTO_SRC_EN(chn) | TSADCV2_AUTO_EN, regs +
+		       TSADCV2_AUTO_CON);
+
+	return 0;
+}
+
+static int rk_tsadcv2_control(void __iomem *regs, bool on)
+{
+	u32 val;
+
+	if (on) {
+		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
+		writel_relaxed(val | TSADCV2_AUTO_EN, regs + TSADCV2_AUTO_CON);
+	} else {
+		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
+		writel_relaxed(val & TSADCV2_AUTO_DISABLE,
+			       regs + TSADCV2_AUTO_CON);
+	}
+
+	return 0;
+}
+
+static void rk_tsadcv2_alarm_temp(int chn, void __iomem *regs,
+				  unsigned long alarm_temp)
+{
+	u32 alarm_value;
+
+	alarm_value = rk_tsadcv2_temp_to_code(alarm_temp);
+
+	writel_relaxed(alarm_value & TSADCV2_DATA_MASK, regs +
+		       TSADCV2_COMP_INT(chn));
+}
+
+static const struct rockchip_tsadc_platform_data rk3288_tsadc_data = {
+	.reset_mode = GPIO, /* default TSHUT via GPIO give PMIC */
+	.temp_passive = 80000,
+	.hw_shut_temp = 120000,
+	.irq_handle = rk_tsadcv2_irq_handle,
+	.initialize = rk_tsadcv2_initialize,
+	.control = rk_tsadcv2_control,
+	.code_to_temp = rk_tsadcv2_code_to_temp,
+	.temp_to_code = rk_tsadcv2_temp_to_code,
+	.set_alarm_temp = rk_tsadcv2_alarm_temp,
+};
+
+static const struct of_device_id of_rockchip_thermal_match[] = {
+	{
+		.compatible = "rockchip,rk3288-tsadc",
+		.data = (void *)&rk3288_tsadc_data,
+	},
+	{ /* end */ },
+};
+MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
+
+static void rockchip_set_alarm_temp(struct rockchip_thermal_data *data,
+				    int alarm_temp)
+{
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+
+	data->alarm_temp = alarm_temp;
+	if (tsadc->set_alarm_temp)
+		tsadc->set_alarm_temp(data->chn, data->regs, alarm_temp);
+}
+
+static int rockchip_thermal_initialize(struct rockchip_thermal_data *data)
+{
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+
+	if (data->chn == RESERVE)
+		return 0;
+
+	if (tsadc->initialize)
+		tsadc->initialize(tsadc->reset_mode, data->chn, data->regs,
+				  data->hw_shut_temp);
+
+	rockchip_set_alarm_temp(data, data->temp_passive);
+
+	return 0;
+}
+
+static void rockchip_thermal_control(struct rockchip_thermal_data *data,
+				     bool on)
+{
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+
+	if (tsadc->control)
+		tsadc->control(data->regs, on);
+
+	if (on) {
+		data->irq_enabled = true;
+		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
+		THERMAL_DEVICE_ENABLED);
+	} else {
+		data->irq_enabled = false;
+		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
+		THERMAL_DEVICE_DISABLED);
+	}
+}
+
+static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
+{
+	struct rockchip_thermal_data *data = dev;
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+	int chn;
+
+	if (tsadc->irq_handle)
+		tsadc->irq_handle(data->regs);
+
+	for (chn = CPU; chn < SENSOR_ID_END; chn++)
+		thermal_zone_device_update(data->tz[chn]);
+
+	return IRQ_HANDLED;
+}
+
+static int rockchip_thermal_set_trips(void *zone, long low, long high)
+{
+	struct rockchip_thermal_data *data = zone;
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+	u32 val;
+
+	low = clamp_val(low, LONG_MIN, LONG_MAX);
+	high = clamp_val(high, LONG_MIN, LONG_MAX);
+
+	/* channel0 is RESERVE, not need to set trips*/
+	if (data->chn == RESERVE)
+		return 0;
+
+	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));
+	if (val == 0)
+		return -EPROBE_DEFER;
+
+	/* Update alarm value to next higher trip point */
+	if (data->alarm_temp == data->temp_passive && val <=
+		tsadc->temp_to_code(data->temp_passive))
+		high = data->hw_shut_temp;
+
+	if (data->alarm_temp >= data->temp_passive && val >
+		tsadc->temp_to_code(data->temp_passive)) {
+		high = data->temp_passive;
+	}
+
+	return 0;
+}
+
+static int rockchip_thermal_get_temp(void *zone, long *out_temp)
+{
+	struct rockchip_thermal_data *data = zone;
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+	u32 val;
+
+	if (data->chn == RESERVE)
+		return 0;
+
+	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));
+
+	if (val < tsadc->temp_to_code(data->hw_shut_temp))
+		*out_temp = 120000;
+	else
+		*out_temp = tsadc->code_to_temp(val);
+
+	return 0;
+}
+
+static int rockchip_configure_from_dt(struct device *dev,
+				      struct device_node *np,
+				      struct rockchip_thermal_data *data)
+{
+	int shut_temp, reset_mode;
+
+	if (of_property_read_u32(np, "hw-shut-temp", &shut_temp)) {
+		dev_warn(dev, "Missing default shutdown temp property\n");
+		data->hw_shut_temp = data->pdata->hw_shut_temp;
+	} else {
+		data->hw_shut_temp = shut_temp;
+	}
+
+	if (of_property_read_u32(np, "tsadc-ht-reset-mode", &reset_mode)) {
+		dev_warn(dev, "Missing default reset mode property\n");
+		data->reset_mode = data->pdata->reset_mode;
+	} else {
+		data->reset_mode = reset_mode;
+	}
+
+	data->temp_passive = data->pdata->temp_passive;
+
+	return 0;
+}
+
+static int rockchip_thermal_probe(struct platform_device *pdev)
+{
+	struct rockchip_thermal_data *data;
+	const struct rockchip_tsadc_platform_data *tsadc;
+	const struct of_device_id *match;
+
+	struct cpumask clip_cpus;
+	struct resource *res;
+	struct device_node *np = pdev->dev.of_node;
+
+	int ret, err, chn;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(struct rockchip_thermal_data),
+			    GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->regs)) {
+		dev_err(&pdev->dev, "Could not get tsadc source, %p\n",
+			data->regs);
+		return PTR_ERR(data->regs);
+	}
+
+	match = of_match_node(of_rockchip_thermal_match, np);
+	if (!match)
+		return -ENXIO;
+	data->pdata = (const struct rockchip_tsadc_platform_data *)match->data;
+	if (!data->pdata)
+		return -EINVAL;
+	tsadc = data->pdata;
+
+	data->clk = devm_clk_get(&pdev->dev, "tsadc");
+	if (IS_ERR(data->clk)) {
+		dev_err(&pdev->dev, "failed to get tsadc clock\n");
+		return PTR_ERR(data->clk);
+	}
+
+	data->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
+	if (IS_ERR(data->pclk)) {
+		dev_err(&pdev->dev, "failed to get tsadc pclk\n");
+		return PTR_ERR(data->pclk);
+	}
+
+	/**
+	* Use a default of 10KHz for the converter clock.
+	* This may become user-configurable in the future.
+	* Need to judge the data->clk is Divided by xin32k.
+	* Need to retry it if the pmic hasn't output the xin32k.
+	*/
+	if (clk_get_rate(data->clk) == 0)
+		return -EPROBE_DEFER;
+	ret = clk_set_rate(data->clk, 10000);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to set tsadc clk rate, %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to enable converter clock\n");
+		goto err_clk;
+	}
+
+	ret = clk_prepare_enable(data->pclk);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to enable pclk\n");
+		goto err_pclk;
+	}
+
+	cpumask_set_cpu(0, &clip_cpus);
+	data->cdev = of_cpufreq_cooling_register(np, &clip_cpus);
+	if (IS_ERR(data->cdev)) {
+		dev_err(&pdev->dev, "failed to register cpufreq cooling device\n");
+		goto disable_clk;
+	}
+
+	data->irq = platform_get_irq(pdev, 0);
+	if (data->irq < 0) {
+		dev_err(&pdev->dev, "no irq resource?\n");
+		goto disable_clk;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, data->irq, NULL,
+					&rockchip_thermal_alarm_irq_thread,
+					IRQF_ONESHOT, "rockchip_thermal",
+					data);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"failed to request tsadc irq: %d\n", ret);
+		goto disable_clk;
+	}
+
+	ret = rockchip_configure_from_dt(&pdev->dev, np, data);
+	if (ret)
+		dev_err(&pdev->dev, "Parsing device tree data error.\n");
+
+	/* The system three Teperture Sensors be registered */
+	for (chn = RESERVE; chn < SENSOR_ID_END; chn++) {
+		data->chn = chn;
+
+		data->tz[chn] = thermal_zone_of_sensor_register(
+				&pdev->dev, data->chn,
+				data, rockchip_thermal_get_temp,
+				NULL,
+				rockchip_thermal_set_trips);
+		if (IS_ERR(data->tz[chn])) {
+			err = PTR_ERR(data->tz[chn]);
+			dev_err(&pdev->dev, "failed to register sensor: %d\n",
+				err);
+			chn--;
+			goto unregister_tzs;
+		}
+
+		platform_set_drvdata(pdev, data);
+
+		rockchip_thermal_initialize(data);
+		rockchip_thermal_control(data, true);
+	}
+	return 0;
+
+unregister_tzs:
+	for (; chn >= RESERVE; chn--)
+		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
+	cpufreq_cooling_unregister(data->cdev);
+
+disable_clk:
+err_pclk:
+	clk_disable_unprepare(data->pclk);
+err_clk:
+	clk_disable_unprepare(data->clk);
+
+	return ret;
+}
+
+static int rockchip_thermal_remove(struct platform_device *pdev)
+{
+	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
+	int chn;
+
+	rockchip_thermal_control(data, false);
+
+	for (; chn >= RESERVE; chn--)
+		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
+	cpufreq_cooling_unregister(data->cdev);
+
+	clk_disable_unprepare(data->clk);
+	clk_disable_unprepare(data->pclk);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int rockchip_thermal_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
+
+	rockchip_thermal_control(data, false);
+
+	clk_disable_unprepare(data->clk);
+	clk_disable_unprepare(data->pclk);
+
+	return 0;
+}
+
+static int rockchip_thermal_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = clk_prepare_enable(data->pclk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret)
+		return ret;
+
+	rockchip_thermal_initialize(data);
+	rockchip_thermal_control(data, true);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(rockchip_thermal_pm_ops,
+			 rockchip_thermal_suspend, rockchip_thermal_resume);
+
+static struct platform_driver rockchip_thermal_driver = {
+	.driver = {
+		   .name = "rockchip-thermal",
+		   .owner = THIS_MODULE,
+		   .pm = &rockchip_thermal_pm_ops,
+		   .of_match_table = of_rockchip_thermal_match,
+		   },
+	.probe = rockchip_thermal_probe,
+	.remove = rockchip_thermal_remove,
+};
+
+module_platform_driver(rockchip_thermal_driver);
+
+MODULE_DESCRIPTION("ROCKCHIP THERMAL Driver");
+MODULE_AUTHOR("Rockchip, Inc.");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:rockchip-thermal");
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v8 1/5] thermal: rockchip: add driver for thermal
@ 2014-10-10 10:19   ` Caesar Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Caesar Wang @ 2014-10-10 10:19 UTC (permalink / raw)
  To: linux-arm-kernel

Thermal is TS-ADC Controller module supports
user-defined mode and automatic mode.

User-defined mode refers,TSADC all the control signals entirely by
software writing to register for direct control.

Automaic mode refers to the module automatically poll TSADC output,
and the results were checked.If you find that the temperature High
in a period of time,an interrupt is generated to the processor
down-measures taken;If the temperature over a period of time High,
the resulting TSHUT gave CRU module,let it reset the entire chip,
or via GPIO give PMIC.

Signed-off-by: zhaoyifeng <zyf@rock-chips.com>
Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
---
 drivers/thermal/Kconfig            |   9 +
 drivers/thermal/Makefile           |   1 +
 drivers/thermal/rockchip_thermal.c | 628 +++++++++++++++++++++++++++++++++++++
 3 files changed, 638 insertions(+)
 create mode 100644 drivers/thermal/rockchip_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index f9a1386..24845ae 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -133,6 +133,15 @@ config SPEAR_THERMAL
 	  Enable this to plug the SPEAr thermal sensor driver into the Linux
 	  thermal framework.
 
+config ROCKCHIP_THERMAL
+	tristate "Rockchip thermal driver"
+	depends on ARCH_ROCKCHIP
+	help
+	  Rockchip thermal driver provides support for Temperature sensor
+	  ADC (TS-ADC) found on Rockchip SoCs. It supports one critical
+	  trip point. Cpufreq is used as the cooling device and will throttle
+	  CPUs when the Temperature crosses the passive trip point.
+
 config RCAR_THERMAL
 	tristate "Renesas R-Car thermal driver"
 	depends on ARCH_SHMOBILE || COMPILE_TEST
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index de0636a..930554f 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -19,6 +19,7 @@ thermal_sys-$(CONFIG_CPU_THERMAL)	+= cpu_cooling.o
 
 # platform thermal drivers
 obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
+obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
 obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
 obj-$(CONFIG_KIRKWOOD_THERMAL)  += kirkwood_thermal.o
 obj-y				+= samsung/
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
new file mode 100644
index 0000000..ceee2c1
--- /dev/null
+++ b/drivers/thermal/rockchip_thermal.c
@@ -0,0 +1,628 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ */
+
+#include <linux/clk.h>
+#include <linux/cpu_cooling.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+/**
+* If the temperature over a period of time High,
+* the resulting TSHUT gave CRU module,let it reset the entire chip,
+* or via GPIO give PMIC.
+*/
+enum reset_mde {
+	CRU = 0,
+	GPIO,
+};
+
+/**
+* The system has three Teperture Sensors.
+* channel0 is reserve,channel1 is for CPU,and
+* channel channel 2 is for GPU.
+*/
+enum sensor_id {
+	RESERVE = 0,
+	CPU,
+	GPU,
+	SENSOR_ID_END,
+};
+
+struct rockchip_thermal_data {
+	const struct rockchip_tsadc_platform_data *pdata;
+	struct thermal_zone_device *tz[SENSOR_ID_END];
+	struct thermal_cooling_device *cdev;
+	void __iomem *regs;
+
+	unsigned long temp_passive;
+	unsigned long hw_shut_temp;
+	unsigned long alarm_temp;
+	bool irq_enabled;
+	int irq;
+	int reset_mode;
+	int chn;
+
+	struct clk *clk;
+	struct clk *pclk;
+};
+
+struct rockchip_tsadc_platform_data {
+	unsigned long temp_passive;
+	unsigned long hw_shut_temp;
+	int reset_mode;
+
+	int (*irq_handle)(void __iomem *reg);
+	int (*initialize)(int reset_mode, int chn, void __iomem *reg,
+			  unsigned long hw_shut_temp);
+	int (*control)(void __iomem *reg, bool on);
+	int (*code_to_temp)(u32 code);
+	u32 (*temp_to_code)(int temp);
+	void (*set_alarm_temp)(int chn, void __iomem *reg,
+			       unsigned long alarm_temp);
+};
+
+/* TSADC V2 Sensor info define: */
+#define TSADCV2_AUTO_CON			0x04
+#define TSADCV2_INT_EN				0x08
+#define TSADCV2_INT_PD				0x0c
+#define TSADCV2_DATA(chn)			(0x20+chn*0x04)
+#define TSADCV2_COMP_INT(chn)		        (0x30+chn*0x04)
+#define TSADCV2_COMP_SHUT(chn)		        (0x40+chn*0x04)
+#define TSADCV2_HIGHT_INT_DEBOUNCE		0x60
+#define TSADCV2_HIGHT_TSHUT_DEBOUNCE		0x64
+#define TSADCV2_AUTO_PERIOD			0x68
+#define TSADCV2_AUTO_PERIOD_HT			0x6c
+
+#define TSADCV2_AUTO_EN				BIT(0)
+#define TSADCV2_AUTO_DISABLE			~BIT(0)
+#define TSADCV2_AUTO_SRC_EN(chn)	        (0xf << (4 + chn))
+#define TSADCV2_AUTO_TSHUT_POLARITY_HIGH	BIT(8)
+
+#define TSADCV2_INT_SRC_EN(chn)			BIT(chn)
+#define TSADCV2_SHUT_2GPIO_SRC_EN(chn)		(0xf << (4 + chn))
+#define TSADCV2_SHUT_2CRU_SRC_EN(chn)		(0xf << (8 + chn))
+
+#define TSADCV2_INT_PD_CLEAR			~BIT(8)
+
+#define TSADCV2_DATA_MASK			0xfff
+#define TSADCV2_HIGHT_INT_DEBOUNCE_TIME		0x0a
+#define TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME	0x0a
+#define TSADCV2_AUTO_PERIOD_TIME		0x03e8
+#define TSADCV2_AUTO_PERIOD_HT_TIME		0x64
+
+struct tsadc_table {
+	unsigned long code;
+	int temp;
+};
+
+static const struct tsadc_table v2_code_table[] = {
+	{TSADCV2_DATA_MASK, -40000},
+	{3800, -40000},
+	{3792, -35000},
+	{3783, -30000},
+	{3774, -25000},
+	{3765, -20000},
+	{3756, -15000},
+	{3747, -10000},
+	{3737, -5000},
+	{3728, 0},
+	{3718, 5000},
+	{3708, 10000},
+	{3698, 15000},
+	{3688, 20000},
+	{3678, 25000},
+	{3667, 30000},
+	{3656, 35000},
+	{3645, 40000},
+	{3634, 45000},
+	{3623, 50000},
+	{3611, 55000},
+	{3600, 60000},
+	{3588, 65000},
+	{3575, 70000},
+	{3563, 75000},
+	{3550, 80000},
+	{3537, 85000},
+	{3524, 90000},
+	{3510, 95000},
+	{3496, 100000},
+	{3482, 105000},
+	{3467, 110000},
+	{3452, 115000},
+	{3437, 120000},
+	{3421, 125000},
+	{0, 125000},
+};
+
+static int rk_tsadcv2_irq_handle(void __iomem *regs)
+{
+	u32 val;
+
+	val = readl_relaxed(regs + TSADCV2_INT_PD);
+	writel_relaxed(val & TSADCV2_INT_PD_CLEAR, regs + TSADCV2_INT_PD);
+
+	return 0;
+}
+
+static u32 rk_tsadcv2_temp_to_code(int temp)
+{
+	int high, low, mid, ret = 0;
+
+	low = 0;
+	high = ARRAY_SIZE(v2_code_table) - 1;
+	mid = (high + low) / 2;
+
+	if (temp < v2_code_table[low].temp || temp > v2_code_table[high].temp) {
+		ret = -ERANGE;
+		goto exit;
+	}
+
+	while (low <= high) {
+		if (temp == v2_code_table[mid].temp)
+			return v2_code_table[mid].code;
+		else if (temp < v2_code_table[mid].temp)
+			high = mid - 1;
+		else
+			low = mid + 1;
+		mid = (low + high) / 2;
+	}
+
+	return 0;
+
+exit:
+	return ret;
+}
+
+static int rk_tsadcv2_code_to_temp(u32 code)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(v2_code_table) - 1; i++) {
+		if (code >= v2_code_table[i].code)
+			return v2_code_table[i].temp;
+	}
+
+	/* No code available,return max temperture */
+	return 125000;
+}
+
+static int rk_tsadcv2_initialize(int reset_mode, int chn, void __iomem *regs,
+				 unsigned long hw_shut_temp)
+{
+	u32 shutdown_value;
+
+	shutdown_value = rk_tsadcv2_temp_to_code(hw_shut_temp);
+
+	/* Enable measurements at ~ 10 Hz */
+	writel_relaxed(0 | TSADCV2_AUTO_TSHUT_POLARITY_HIGH, regs +
+		       TSADCV2_AUTO_CON);
+	writel_relaxed(TSADCV2_AUTO_PERIOD_TIME, regs + TSADCV2_AUTO_PERIOD);
+	writel_relaxed(TSADCV2_AUTO_PERIOD_HT_TIME, regs +
+		       TSADCV2_AUTO_PERIOD_HT);
+	writel_relaxed(shutdown_value, regs + TSADCV2_COMP_SHUT(chn));
+	writel_relaxed(TSADCV2_HIGHT_INT_DEBOUNCE_TIME, regs +
+		       TSADCV2_HIGHT_INT_DEBOUNCE);
+	writel_relaxed(TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME, regs +
+		       TSADCV2_HIGHT_TSHUT_DEBOUNCE);
+
+	if (reset_mode == GPIO)
+		writel_relaxed(TSADCV2_SHUT_2GPIO_SRC_EN(chn) |
+			       TSADCV2_INT_SRC_EN(chn), regs +
+			       TSADCV2_INT_EN);
+	else
+		writel_relaxed(TSADCV2_SHUT_2CRU_SRC_EN(chn) |
+			       TSADCV2_INT_SRC_EN(chn) , regs +
+			       TSADCV2_INT_EN);
+
+	writel_relaxed(TSADCV2_AUTO_SRC_EN(chn) | TSADCV2_AUTO_EN, regs +
+		       TSADCV2_AUTO_CON);
+
+	return 0;
+}
+
+static int rk_tsadcv2_control(void __iomem *regs, bool on)
+{
+	u32 val;
+
+	if (on) {
+		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
+		writel_relaxed(val | TSADCV2_AUTO_EN, regs + TSADCV2_AUTO_CON);
+	} else {
+		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
+		writel_relaxed(val & TSADCV2_AUTO_DISABLE,
+			       regs + TSADCV2_AUTO_CON);
+	}
+
+	return 0;
+}
+
+static void rk_tsadcv2_alarm_temp(int chn, void __iomem *regs,
+				  unsigned long alarm_temp)
+{
+	u32 alarm_value;
+
+	alarm_value = rk_tsadcv2_temp_to_code(alarm_temp);
+
+	writel_relaxed(alarm_value & TSADCV2_DATA_MASK, regs +
+		       TSADCV2_COMP_INT(chn));
+}
+
+static const struct rockchip_tsadc_platform_data rk3288_tsadc_data = {
+	.reset_mode = GPIO, /* default TSHUT via GPIO give PMIC */
+	.temp_passive = 80000,
+	.hw_shut_temp = 120000,
+	.irq_handle = rk_tsadcv2_irq_handle,
+	.initialize = rk_tsadcv2_initialize,
+	.control = rk_tsadcv2_control,
+	.code_to_temp = rk_tsadcv2_code_to_temp,
+	.temp_to_code = rk_tsadcv2_temp_to_code,
+	.set_alarm_temp = rk_tsadcv2_alarm_temp,
+};
+
+static const struct of_device_id of_rockchip_thermal_match[] = {
+	{
+		.compatible = "rockchip,rk3288-tsadc",
+		.data = (void *)&rk3288_tsadc_data,
+	},
+	{ /* end */ },
+};
+MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
+
+static void rockchip_set_alarm_temp(struct rockchip_thermal_data *data,
+				    int alarm_temp)
+{
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+
+	data->alarm_temp = alarm_temp;
+	if (tsadc->set_alarm_temp)
+		tsadc->set_alarm_temp(data->chn, data->regs, alarm_temp);
+}
+
+static int rockchip_thermal_initialize(struct rockchip_thermal_data *data)
+{
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+
+	if (data->chn == RESERVE)
+		return 0;
+
+	if (tsadc->initialize)
+		tsadc->initialize(tsadc->reset_mode, data->chn, data->regs,
+				  data->hw_shut_temp);
+
+	rockchip_set_alarm_temp(data, data->temp_passive);
+
+	return 0;
+}
+
+static void rockchip_thermal_control(struct rockchip_thermal_data *data,
+				     bool on)
+{
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+
+	if (tsadc->control)
+		tsadc->control(data->regs, on);
+
+	if (on) {
+		data->irq_enabled = true;
+		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
+		THERMAL_DEVICE_ENABLED);
+	} else {
+		data->irq_enabled = false;
+		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
+		THERMAL_DEVICE_DISABLED);
+	}
+}
+
+static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
+{
+	struct rockchip_thermal_data *data = dev;
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+	int chn;
+
+	if (tsadc->irq_handle)
+		tsadc->irq_handle(data->regs);
+
+	for (chn = CPU; chn < SENSOR_ID_END; chn++)
+		thermal_zone_device_update(data->tz[chn]);
+
+	return IRQ_HANDLED;
+}
+
+static int rockchip_thermal_set_trips(void *zone, long low, long high)
+{
+	struct rockchip_thermal_data *data = zone;
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+	u32 val;
+
+	low = clamp_val(low, LONG_MIN, LONG_MAX);
+	high = clamp_val(high, LONG_MIN, LONG_MAX);
+
+	/* channel0 is RESERVE, not need to set trips*/
+	if (data->chn == RESERVE)
+		return 0;
+
+	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));
+	if (val == 0)
+		return -EPROBE_DEFER;
+
+	/* Update alarm value to next higher trip point */
+	if (data->alarm_temp == data->temp_passive && val <=
+		tsadc->temp_to_code(data->temp_passive))
+		high = data->hw_shut_temp;
+
+	if (data->alarm_temp >= data->temp_passive && val >
+		tsadc->temp_to_code(data->temp_passive)) {
+		high = data->temp_passive;
+	}
+
+	return 0;
+}
+
+static int rockchip_thermal_get_temp(void *zone, long *out_temp)
+{
+	struct rockchip_thermal_data *data = zone;
+	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+	u32 val;
+
+	if (data->chn == RESERVE)
+		return 0;
+
+	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));
+
+	if (val < tsadc->temp_to_code(data->hw_shut_temp))
+		*out_temp = 120000;
+	else
+		*out_temp = tsadc->code_to_temp(val);
+
+	return 0;
+}
+
+static int rockchip_configure_from_dt(struct device *dev,
+				      struct device_node *np,
+				      struct rockchip_thermal_data *data)
+{
+	int shut_temp, reset_mode;
+
+	if (of_property_read_u32(np, "hw-shut-temp", &shut_temp)) {
+		dev_warn(dev, "Missing default shutdown temp property\n");
+		data->hw_shut_temp = data->pdata->hw_shut_temp;
+	} else {
+		data->hw_shut_temp = shut_temp;
+	}
+
+	if (of_property_read_u32(np, "tsadc-ht-reset-mode", &reset_mode)) {
+		dev_warn(dev, "Missing default reset mode property\n");
+		data->reset_mode = data->pdata->reset_mode;
+	} else {
+		data->reset_mode = reset_mode;
+	}
+
+	data->temp_passive = data->pdata->temp_passive;
+
+	return 0;
+}
+
+static int rockchip_thermal_probe(struct platform_device *pdev)
+{
+	struct rockchip_thermal_data *data;
+	const struct rockchip_tsadc_platform_data *tsadc;
+	const struct of_device_id *match;
+
+	struct cpumask clip_cpus;
+	struct resource *res;
+	struct device_node *np = pdev->dev.of_node;
+
+	int ret, err, chn;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(struct rockchip_thermal_data),
+			    GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->regs)) {
+		dev_err(&pdev->dev, "Could not get tsadc source, %p\n",
+			data->regs);
+		return PTR_ERR(data->regs);
+	}
+
+	match = of_match_node(of_rockchip_thermal_match, np);
+	if (!match)
+		return -ENXIO;
+	data->pdata = (const struct rockchip_tsadc_platform_data *)match->data;
+	if (!data->pdata)
+		return -EINVAL;
+	tsadc = data->pdata;
+
+	data->clk = devm_clk_get(&pdev->dev, "tsadc");
+	if (IS_ERR(data->clk)) {
+		dev_err(&pdev->dev, "failed to get tsadc clock\n");
+		return PTR_ERR(data->clk);
+	}
+
+	data->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
+	if (IS_ERR(data->pclk)) {
+		dev_err(&pdev->dev, "failed to get tsadc pclk\n");
+		return PTR_ERR(data->pclk);
+	}
+
+	/**
+	* Use a default of 10KHz for the converter clock.
+	* This may become user-configurable in the future.
+	* Need to judge the data->clk is Divided by xin32k.
+	* Need to retry it if the pmic hasn't output the xin32k.
+	*/
+	if (clk_get_rate(data->clk) == 0)
+		return -EPROBE_DEFER;
+	ret = clk_set_rate(data->clk, 10000);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to set tsadc clk rate, %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to enable converter clock\n");
+		goto err_clk;
+	}
+
+	ret = clk_prepare_enable(data->pclk);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to enable pclk\n");
+		goto err_pclk;
+	}
+
+	cpumask_set_cpu(0, &clip_cpus);
+	data->cdev = of_cpufreq_cooling_register(np, &clip_cpus);
+	if (IS_ERR(data->cdev)) {
+		dev_err(&pdev->dev, "failed to register cpufreq cooling device\n");
+		goto disable_clk;
+	}
+
+	data->irq = platform_get_irq(pdev, 0);
+	if (data->irq < 0) {
+		dev_err(&pdev->dev, "no irq resource?\n");
+		goto disable_clk;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, data->irq, NULL,
+					&rockchip_thermal_alarm_irq_thread,
+					IRQF_ONESHOT, "rockchip_thermal",
+					data);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"failed to request tsadc irq: %d\n", ret);
+		goto disable_clk;
+	}
+
+	ret = rockchip_configure_from_dt(&pdev->dev, np, data);
+	if (ret)
+		dev_err(&pdev->dev, "Parsing device tree data error.\n");
+
+	/* The system three Teperture Sensors be registered */
+	for (chn = RESERVE; chn < SENSOR_ID_END; chn++) {
+		data->chn = chn;
+
+		data->tz[chn] = thermal_zone_of_sensor_register(
+				&pdev->dev, data->chn,
+				data, rockchip_thermal_get_temp,
+				NULL,
+				rockchip_thermal_set_trips);
+		if (IS_ERR(data->tz[chn])) {
+			err = PTR_ERR(data->tz[chn]);
+			dev_err(&pdev->dev, "failed to register sensor: %d\n",
+				err);
+			chn--;
+			goto unregister_tzs;
+		}
+
+		platform_set_drvdata(pdev, data);
+
+		rockchip_thermal_initialize(data);
+		rockchip_thermal_control(data, true);
+	}
+	return 0;
+
+unregister_tzs:
+	for (; chn >= RESERVE; chn--)
+		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
+	cpufreq_cooling_unregister(data->cdev);
+
+disable_clk:
+err_pclk:
+	clk_disable_unprepare(data->pclk);
+err_clk:
+	clk_disable_unprepare(data->clk);
+
+	return ret;
+}
+
+static int rockchip_thermal_remove(struct platform_device *pdev)
+{
+	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
+	int chn;
+
+	rockchip_thermal_control(data, false);
+
+	for (; chn >= RESERVE; chn--)
+		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
+	cpufreq_cooling_unregister(data->cdev);
+
+	clk_disable_unprepare(data->clk);
+	clk_disable_unprepare(data->pclk);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int rockchip_thermal_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
+
+	rockchip_thermal_control(data, false);
+
+	clk_disable_unprepare(data->clk);
+	clk_disable_unprepare(data->pclk);
+
+	return 0;
+}
+
+static int rockchip_thermal_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = clk_prepare_enable(data->pclk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret)
+		return ret;
+
+	rockchip_thermal_initialize(data);
+	rockchip_thermal_control(data, true);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(rockchip_thermal_pm_ops,
+			 rockchip_thermal_suspend, rockchip_thermal_resume);
+
+static struct platform_driver rockchip_thermal_driver = {
+	.driver = {
+		   .name = "rockchip-thermal",
+		   .owner = THIS_MODULE,
+		   .pm = &rockchip_thermal_pm_ops,
+		   .of_match_table = of_rockchip_thermal_match,
+		   },
+	.probe = rockchip_thermal_probe,
+	.remove = rockchip_thermal_remove,
+};
+
+module_platform_driver(rockchip_thermal_driver);
+
+MODULE_DESCRIPTION("ROCKCHIP THERMAL Driver");
+MODULE_AUTHOR("Rockchip, Inc.");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:rockchip-thermal");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v8 2/5] dt-bindings: document Rockchip thermal
  2014-10-10 10:19 ` Caesar Wang
@ 2014-10-10 10:19   ` Caesar Wang
  -1 siblings, 0 replies; 14+ messages in thread
From: Caesar Wang @ 2014-10-10 10:19 UTC (permalink / raw)
  To: heiko, rui.zhang, edubezval, arnd
  Cc: zyf, dianders, linux-rockchip, linux-kernel, linux-pm,
	linux-arm-kernel, devicetree, linux-doc, cf, dmitry.torokhov,
	dbasehore, huangtao, cjf, zhengsq, Caesar Wang

This add the necessary binding documentation for the thermal
found on Rockchip SoCs

Signed-off-by: zhaoyifeng <zyf@rock-chips.com>
Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
---
 .../bindings/thermal/rockchip-thermal.txt          | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/rockchip-thermal.txt

diff --git a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
new file mode 100644
index 0000000..d5ff410
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
@@ -0,0 +1,45 @@
+* Temperature Sensor ADC (TSADC) on rockchip SoCs
+
+Required properties:
+- compatible: "rockchip,rk3288-tsadc"
+- reg: physical base address of the controller and length of memory mapped
+       region.
+- interrupts: The interrupt number to the cpu. The interrupt specifier format
+	      depends on the interrupt controller.
+- clocks: Must contain an entry for each entry in clock-names.
+- clock-names: Shall be "tsadc" for the converter-clock, and "apb_pclk" for
+	       the peripheral clock.
+- #thermal-sensor-cells: Should be 1. See ./thermal.txt for a description.
+
+Exiample:
+tsadc: tsadc@ff280000 {
+	compatible = "rockchip,rk3288-tsadc";
+	reg = <0xff280000 0x100>;
+	interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&cru SCLK_TSADC>, <&cru PCLK_TSADC>;
+	clock-names = "tsadc", "apb_pclk";
+};
+
+Example: referring to thermal sensors:
+thermal-zones {
+       cpu_thermal: cpu_thermal {
+	        polling-delay-passive = <500>; /* milliseconds */
+	        polling-delay = <1000>; /* milliseconds */
+
+			        /* sensor	ID */
+	        thermal-sensors = <&tsadc	1>;
+
+	        trips {
+		        cpu_alert0: cpu_alert {
+			        temperature = <80000>; /* millicelsius */
+			        hysteresis = <2000>; /* millicelsius */
+			        type = "passive";
+		        };
+		        cpu_crit: cpu_crit {
+			        temperature = <125000>; /* millicelsius */
+			        hysteresis = <2000>; /* millicelsius */
+			    type = "critical";
+		        };
+	        };
+       };
+};
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v8 2/5] dt-bindings: document Rockchip thermal
@ 2014-10-10 10:19   ` Caesar Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Caesar Wang @ 2014-10-10 10:19 UTC (permalink / raw)
  To: linux-arm-kernel

This add the necessary binding documentation for the thermal
found on Rockchip SoCs

Signed-off-by: zhaoyifeng <zyf@rock-chips.com>
Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
---
 .../bindings/thermal/rockchip-thermal.txt          | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/rockchip-thermal.txt

diff --git a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
new file mode 100644
index 0000000..d5ff410
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
@@ -0,0 +1,45 @@
+* Temperature Sensor ADC (TSADC) on rockchip SoCs
+
+Required properties:
+- compatible: "rockchip,rk3288-tsadc"
+- reg: physical base address of the controller and length of memory mapped
+       region.
+- interrupts: The interrupt number to the cpu. The interrupt specifier format
+	      depends on the interrupt controller.
+- clocks: Must contain an entry for each entry in clock-names.
+- clock-names: Shall be "tsadc" for the converter-clock, and "apb_pclk" for
+	       the peripheral clock.
+- #thermal-sensor-cells: Should be 1. See ./thermal.txt for a description.
+
+Exiample:
+tsadc: tsadc at ff280000 {
+	compatible = "rockchip,rk3288-tsadc";
+	reg = <0xff280000 0x100>;
+	interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&cru SCLK_TSADC>, <&cru PCLK_TSADC>;
+	clock-names = "tsadc", "apb_pclk";
+};
+
+Example: referring to thermal sensors:
+thermal-zones {
+       cpu_thermal: cpu_thermal {
+	        polling-delay-passive = <500>; /* milliseconds */
+	        polling-delay = <1000>; /* milliseconds */
+
+			        /* sensor	ID */
+	        thermal-sensors = <&tsadc	1>;
+
+	        trips {
+		        cpu_alert0: cpu_alert {
+			        temperature = <80000>; /* millicelsius */
+			        hysteresis = <2000>; /* millicelsius */
+			        type = "passive";
+		        };
+		        cpu_crit: cpu_crit {
+			        temperature = <125000>; /* millicelsius */
+			        hysteresis = <2000>; /* millicelsius */
+			    type = "critical";
+		        };
+	        };
+       };
+};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v8 3/5] ARM: dts: add RK3288 Thermal data
  2014-10-10 10:19 ` Caesar Wang
@ 2014-10-10 10:19   ` Caesar Wang
  -1 siblings, 0 replies; 14+ messages in thread
From: Caesar Wang @ 2014-10-10 10:19 UTC (permalink / raw)
  To: heiko, rui.zhang, edubezval, arnd
  Cc: zyf, dianders, linux-rockchip, linux-kernel, linux-pm,
	linux-arm-kernel, devicetree, linux-doc, cf, dmitry.torokhov,
	dbasehore, huangtao, cjf, zhengsq, Caesar Wang

This patch changes a dtsi file to contain the thermal data
on RK3288 and later SoCs. This data will
enable a thermal shutdown over 125C.

Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
---
 arch/arm/boot/dts/rk3288-thermal.dtsi | 57 +++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 arch/arm/boot/dts/rk3288-thermal.dtsi

diff --git a/arch/arm/boot/dts/rk3288-thermal.dtsi b/arch/arm/boot/dts/rk3288-thermal.dtsi
new file mode 100644
index 0000000..c68c090
--- /dev/null
+++ b/arch/arm/boot/dts/rk3288-thermal.dtsi
@@ -0,0 +1,57 @@
+/*
+ * Device Tree Source for RK3288 SoC thermal
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <dt-bindings/thermal/thermal.h>
+
+reserve_thermal: reserve_thermal {
+	polling-delay-passive = <500>; /* milliseconds */
+	polling-delay = <1000>; /* milliseconds */
+
+			/* sensor	ID */
+	thermal-sensors = <&tsadc	0>;
+
+};
+
+cpu_thermal: cpu_thermal {
+	polling-delay-passive = <500>; /* milliseconds */
+	polling-delay = <1000>; /* milliseconds */
+
+			/* sensor	ID */
+	thermal-sensors = <&tsadc	1>;
+
+	trips {
+		cpu_alert0: cpu_alert0 {
+			temperature = <80000>; /* millicelsius */
+			hysteresis = <2000>; /* millicelsius */
+			type = "passive";
+		};
+		cpu_crit: cpu_crit {
+			temperature = <125000>; /* millicelsius */
+			hysteresis = <2000>; /* millicelsius */
+			type = "critical";
+		};
+	};
+};
+
+gpu_thermal: gpu_thermal {
+	polling-delay-passive = <500>; /* milliseconds */
+	polling-delay = <1000>; /* milliseconds */
+
+			/* sensor	ID */
+	thermal-sensors = <&tsadc	2>;
+
+	trips {
+		gpu_crit: gpu_crit {
+			temperature = <125000>; /* millicelsius */
+			hysteresis = <2000>; /* millicelsius */
+			type = "critical";
+		};
+	};
+};
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v8 3/5] ARM: dts: add RK3288 Thermal data
@ 2014-10-10 10:19   ` Caesar Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Caesar Wang @ 2014-10-10 10:19 UTC (permalink / raw)
  To: linux-arm-kernel

This patch changes a dtsi file to contain the thermal data
on RK3288 and later SoCs. This data will
enable a thermal shutdown over 125C.

Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
---
 arch/arm/boot/dts/rk3288-thermal.dtsi | 57 +++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 arch/arm/boot/dts/rk3288-thermal.dtsi

diff --git a/arch/arm/boot/dts/rk3288-thermal.dtsi b/arch/arm/boot/dts/rk3288-thermal.dtsi
new file mode 100644
index 0000000..c68c090
--- /dev/null
+++ b/arch/arm/boot/dts/rk3288-thermal.dtsi
@@ -0,0 +1,57 @@
+/*
+ * Device Tree Source for RK3288 SoC thermal
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <dt-bindings/thermal/thermal.h>
+
+reserve_thermal: reserve_thermal {
+	polling-delay-passive = <500>; /* milliseconds */
+	polling-delay = <1000>; /* milliseconds */
+
+			/* sensor	ID */
+	thermal-sensors = <&tsadc	0>;
+
+};
+
+cpu_thermal: cpu_thermal {
+	polling-delay-passive = <500>; /* milliseconds */
+	polling-delay = <1000>; /* milliseconds */
+
+			/* sensor	ID */
+	thermal-sensors = <&tsadc	1>;
+
+	trips {
+		cpu_alert0: cpu_alert0 {
+			temperature = <80000>; /* millicelsius */
+			hysteresis = <2000>; /* millicelsius */
+			type = "passive";
+		};
+		cpu_crit: cpu_crit {
+			temperature = <125000>; /* millicelsius */
+			hysteresis = <2000>; /* millicelsius */
+			type = "critical";
+		};
+	};
+};
+
+gpu_thermal: gpu_thermal {
+	polling-delay-passive = <500>; /* milliseconds */
+	polling-delay = <1000>; /* milliseconds */
+
+			/* sensor	ID */
+	thermal-sensors = <&tsadc	2>;
+
+	trips {
+		gpu_crit: gpu_crit {
+			temperature = <125000>; /* millicelsius */
+			hysteresis = <2000>; /* millicelsius */
+			type = "critical";
+		};
+	};
+};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v8 1/5] thermal: rockchip: add driver for thermal
  2014-10-10 10:19   ` Caesar Wang
@ 2014-10-10 17:46     ` Dmitry Torokhov
  -1 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2014-10-10 17:46 UTC (permalink / raw)
  To: Caesar Wang
  Cc: heiko, rui.zhang, edubezval, arnd, zyf, dianders, linux-rockchip,
	linux-kernel, linux-pm, linux-arm-kernel, devicetree, linux-doc,
	cf, dbasehore, huangtao, cjf, zhengsq

Hi Caesar,

On Fri, Oct 10, 2014 at 06:19:37PM +0800, Caesar Wang wrote:
> Thermal is TS-ADC Controller module supports
> user-defined mode and automatic mode.
> 
> User-defined mode refers,TSADC all the control signals entirely by
> software writing to register for direct control.
> 
> Automaic mode refers to the module automatically poll TSADC output,
> and the results were checked.If you find that the temperature High
> in a period of time,an interrupt is generated to the processor
> down-measures taken;If the temperature over a period of time High,
> the resulting TSHUT gave CRU module,let it reset the entire chip,
> or via GPIO give PMIC.
> 
> Signed-off-by: zhaoyifeng <zyf@rock-chips.com>
> Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
> ---
>  drivers/thermal/Kconfig            |   9 +
>  drivers/thermal/Makefile           |   1 +
>  drivers/thermal/rockchip_thermal.c | 628 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 638 insertions(+)
>  create mode 100644 drivers/thermal/rockchip_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index f9a1386..24845ae 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -133,6 +133,15 @@ config SPEAR_THERMAL
>  	  Enable this to plug the SPEAr thermal sensor driver into the Linux
>  	  thermal framework.
>  
> +config ROCKCHIP_THERMAL
> +	tristate "Rockchip thermal driver"
> +	depends on ARCH_ROCKCHIP
> +	help
> +	  Rockchip thermal driver provides support for Temperature sensor
> +	  ADC (TS-ADC) found on Rockchip SoCs. It supports one critical
> +	  trip point. Cpufreq is used as the cooling device and will throttle
> +	  CPUs when the Temperature crosses the passive trip point.
> +
>  config RCAR_THERMAL
>  	tristate "Renesas R-Car thermal driver"
>  	depends on ARCH_SHMOBILE || COMPILE_TEST
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index de0636a..930554f 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -19,6 +19,7 @@ thermal_sys-$(CONFIG_CPU_THERMAL)	+= cpu_cooling.o
>  
>  # platform thermal drivers
>  obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
> +obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
>  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
>  obj-$(CONFIG_KIRKWOOD_THERMAL)  += kirkwood_thermal.o
>  obj-y				+= samsung/
> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> new file mode 100644
> index 0000000..ceee2c1
> --- /dev/null
> +++ b/drivers/thermal/rockchip_thermal.c
> @@ -0,0 +1,628 @@
> +/*
> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/cpu_cooling.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/thermal.h>
> +
> +/**
> +* If the temperature over a period of time High,
> +* the resulting TSHUT gave CRU module,let it reset the entire chip,
> +* or via GPIO give PMIC.
> +*/
> +enum reset_mde {

Can we spare one more letter? ;)

> +	CRU = 0,
> +	GPIO,
> +};
> +
> +/**
> +* The system has three Teperture Sensors.
> +* channel0 is reserve,channel1 is for CPU,and
> +* channel channel 2 is for GPU.
> +*/
> +enum sensor_id {
> +	RESERVE = 0,
> +	CPU,
> +	GPU,
> +	SENSOR_ID_END,
> +};
> +
> +struct rockchip_thermal_data {
> +	const struct rockchip_tsadc_platform_data *pdata;
> +	struct thermal_zone_device *tz[SENSOR_ID_END];
> +	struct thermal_cooling_device *cdev;
> +	void __iomem *regs;
> +
> +	unsigned long temp_passive;
> +	unsigned long hw_shut_temp;
> +	unsigned long alarm_temp;
> +	bool irq_enabled;
> +	int irq;
> +	int reset_mode;
> +	int chn;
> +
> +	struct clk *clk;
> +	struct clk *pclk;
> +};
> +
> +struct rockchip_tsadc_platform_data {
> +	unsigned long temp_passive;
> +	unsigned long hw_shut_temp;
> +	int reset_mode;
> +
> +	int (*irq_handle)(void __iomem *reg);

Why does it return int and not void? What is the return value of this?

> +	int (*initialize)(int reset_mode, int chn, void __iomem *reg,
Why does it return int and not void? What is the return value of this?> +			  unsigned long hw_shut_temp);
> +	int (*control)(void __iomem *reg, bool on);
> +	int (*code_to_temp)(u32 code);
> +	u32 (*temp_to_code)(int temp);
> +	void (*set_alarm_temp)(int chn, void __iomem *reg,
> +			       unsigned long alarm_temp);
> +};
> +
> +/* TSADC V2 Sensor info define: */
> +#define TSADCV2_AUTO_CON			0x04
> +#define TSADCV2_INT_EN				0x08
> +#define TSADCV2_INT_PD				0x0c
> +#define TSADCV2_DATA(chn)			(0x20+chn*0x04)

Unsafe macros. You want to enclose argument in parenthesis.

> +#define TSADCV2_COMP_INT(chn)		        (0x30+chn*0x04)
> +#define TSADCV2_COMP_SHUT(chn)		        (0x40+chn*0x04)
> +#define TSADCV2_HIGHT_INT_DEBOUNCE		0x60
> +#define TSADCV2_HIGHT_TSHUT_DEBOUNCE		0x64
> +#define TSADCV2_AUTO_PERIOD			0x68
> +#define TSADCV2_AUTO_PERIOD_HT			0x6c
> +
> +#define TSADCV2_AUTO_EN				BIT(0)
> +#define TSADCV2_AUTO_DISABLE			~BIT(0)
> +#define TSADCV2_AUTO_SRC_EN(chn)	        (0xf << (4 + chn))
> +#define TSADCV2_AUTO_TSHUT_POLARITY_HIGH	BIT(8)
> +
> +#define TSADCV2_INT_SRC_EN(chn)			BIT(chn)
> +#define TSADCV2_SHUT_2GPIO_SRC_EN(chn)		(0xf << (4 + chn))
> +#define TSADCV2_SHUT_2CRU_SRC_EN(chn)		(0xf << (8 + chn))
> +
> +#define TSADCV2_INT_PD_CLEAR			~BIT(8)
> +
> +#define TSADCV2_DATA_MASK			0xfff
> +#define TSADCV2_HIGHT_INT_DEBOUNCE_TIME		0x0a
> +#define TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME	0x0a
> +#define TSADCV2_AUTO_PERIOD_TIME		0x03e8
> +#define TSADCV2_AUTO_PERIOD_HT_TIME		0x64
> +
> +struct tsadc_table {
> +	unsigned long code;
> +	int temp;
> +};
> +
> +static const struct tsadc_table v2_code_table[] = {
> +	{TSADCV2_DATA_MASK, -40000},
> +	{3800, -40000},
> +	{3792, -35000},
> +	{3783, -30000},
> +	{3774, -25000},
> +	{3765, -20000},
> +	{3756, -15000},
> +	{3747, -10000},
> +	{3737, -5000},
> +	{3728, 0},
> +	{3718, 5000},
> +	{3708, 10000},
> +	{3698, 15000},
> +	{3688, 20000},
> +	{3678, 25000},
> +	{3667, 30000},
> +	{3656, 35000},
> +	{3645, 40000},
> +	{3634, 45000},
> +	{3623, 50000},
> +	{3611, 55000},
> +	{3600, 60000},
> +	{3588, 65000},
> +	{3575, 70000},
> +	{3563, 75000},
> +	{3550, 80000},
> +	{3537, 85000},
> +	{3524, 90000},
> +	{3510, 95000},
> +	{3496, 100000},
> +	{3482, 105000},
> +	{3467, 110000},
> +	{3452, 115000},
> +	{3437, 120000},
> +	{3421, 125000},
> +	{0, 125000},
> +};
> +
> +static int rk_tsadcv2_irq_handle(void __iomem *regs)
> +{
> +	u32 val;
> +
> +	val = readl_relaxed(regs + TSADCV2_INT_PD);
> +	writel_relaxed(val & TSADCV2_INT_PD_CLEAR, regs + TSADCV2_INT_PD);
> +
> +	return 0;
> +}
> +
> +static u32 rk_tsadcv2_temp_to_code(int temp)
> +{
> +	int high, low, mid, ret = 0;
> +
> +	low = 0;
> +	high = ARRAY_SIZE(v2_code_table) - 1;
> +	mid = (high + low) / 2;
> +
> +	if (temp < v2_code_table[low].temp || temp > v2_code_table[high].temp) {
> +		ret = -ERANGE;

The function returns u32, you can't return -ERANGE here.

> +		goto exit;
> +	}
> +
> +	while (low <= high) {
> +		if (temp == v2_code_table[mid].temp)
> +			return v2_code_table[mid].code;
> +		else if (temp < v2_code_table[mid].temp)
> +			high = mid - 1;
> +		else
> +			low = mid + 1;
> +		mid = (low + high) / 2;
> +	}

Hmm, you optimized temp_to_code but not code_to_temp, why?

> +
> +	return 0;
> +
> +exit:
> +	return ret;
> +}
> +
> +static int rk_tsadcv2_code_to_temp(u32 code)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(v2_code_table) - 1; i++) {
> +		if (code >= v2_code_table[i].code)
> +			return v2_code_table[i].temp;
> +	}
> +
> +	/* No code available,return max temperture */
> +	return 125000;
> +}
> +
> +static int rk_tsadcv2_initialize(int reset_mode, int chn, void __iomem *regs,
> +				 unsigned long hw_shut_temp)
> +{
> +	u32 shutdown_value;
> +
> +	shutdown_value = rk_tsadcv2_temp_to_code(hw_shut_temp);
> +
> +	/* Enable measurements at ~ 10 Hz */
> +	writel_relaxed(0 | TSADCV2_AUTO_TSHUT_POLARITY_HIGH, regs +
> +		       TSADCV2_AUTO_CON);
> +	writel_relaxed(TSADCV2_AUTO_PERIOD_TIME, regs + TSADCV2_AUTO_PERIOD);
> +	writel_relaxed(TSADCV2_AUTO_PERIOD_HT_TIME, regs +
> +		       TSADCV2_AUTO_PERIOD_HT);
> +	writel_relaxed(shutdown_value, regs + TSADCV2_COMP_SHUT(chn));
> +	writel_relaxed(TSADCV2_HIGHT_INT_DEBOUNCE_TIME, regs +
> +		       TSADCV2_HIGHT_INT_DEBOUNCE);
> +	writel_relaxed(TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME, regs +
> +		       TSADCV2_HIGHT_TSHUT_DEBOUNCE);
> +
> +	if (reset_mode == GPIO)
> +		writel_relaxed(TSADCV2_SHUT_2GPIO_SRC_EN(chn) |
> +			       TSADCV2_INT_SRC_EN(chn), regs +
> +			       TSADCV2_INT_EN);
> +	else
> +		writel_relaxed(TSADCV2_SHUT_2CRU_SRC_EN(chn) |
> +			       TSADCV2_INT_SRC_EN(chn) , regs +
> +			       TSADCV2_INT_EN);
> +
> +	writel_relaxed(TSADCV2_AUTO_SRC_EN(chn) | TSADCV2_AUTO_EN, regs +
> +		       TSADCV2_AUTO_CON);
> +
> +	return 0;
> +}
> +
> +static int rk_tsadcv2_control(void __iomem *regs, bool on)
> +{
> +	u32 val;
> +
> +	if (on) {
> +		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
> +		writel_relaxed(val | TSADCV2_AUTO_EN, regs + TSADCV2_AUTO_CON);
> +	} else {
> +		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
> +		writel_relaxed(val & TSADCV2_AUTO_DISABLE,
> +			       regs + TSADCV2_AUTO_CON);
> +	}
> +
> +	return 0;
> +}
> +
> +static void rk_tsadcv2_alarm_temp(int chn, void __iomem *regs,
> +				  unsigned long alarm_temp)
> +{
> +	u32 alarm_value;
> +
> +	alarm_value = rk_tsadcv2_temp_to_code(alarm_temp);
> +
> +	writel_relaxed(alarm_value & TSADCV2_DATA_MASK, regs +
> +		       TSADCV2_COMP_INT(chn));
> +}
> +
> +static const struct rockchip_tsadc_platform_data rk3288_tsadc_data = {
> +	.reset_mode = GPIO, /* default TSHUT via GPIO give PMIC */
> +	.temp_passive = 80000,
> +	.hw_shut_temp = 120000,
> +	.irq_handle = rk_tsadcv2_irq_handle,
> +	.initialize = rk_tsadcv2_initialize,
> +	.control = rk_tsadcv2_control,
> +	.code_to_temp = rk_tsadcv2_code_to_temp,
> +	.temp_to_code = rk_tsadcv2_temp_to_code,
> +	.set_alarm_temp = rk_tsadcv2_alarm_temp,
> +};
> +
> +static const struct of_device_id of_rockchip_thermal_match[] = {
> +	{
> +		.compatible = "rockchip,rk3288-tsadc",
> +		.data = (void *)&rk3288_tsadc_data,
> +	},
> +	{ /* end */ },
> +};
> +MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
> +
> +static void rockchip_set_alarm_temp(struct rockchip_thermal_data *data,
> +				    int alarm_temp)
> +{
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +
> +	data->alarm_temp = alarm_temp;
> +	if (tsadc->set_alarm_temp)
> +		tsadc->set_alarm_temp(data->chn, data->regs, alarm_temp);
> +}
> +
> +static int rockchip_thermal_initialize(struct rockchip_thermal_data *data)
> +{
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +
> +	if (data->chn == RESERVE)
> +		return 0;
> +
> +	if (tsadc->initialize)
> +		tsadc->initialize(tsadc->reset_mode, data->chn, data->regs,
> +				  data->hw_shut_temp);
> +
> +	rockchip_set_alarm_temp(data, data->temp_passive);
> +
> +	return 0;
> +}
> +
> +static void rockchip_thermal_control(struct rockchip_thermal_data *data,
> +				     bool on)
> +{
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +
> +	if (tsadc->control)
> +		tsadc->control(data->regs, on);
> +
> +	if (on) {
> +		data->irq_enabled = true;
> +		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
> +		THERMAL_DEVICE_ENABLED);
> +	} else {
> +		data->irq_enabled = false;
> +		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
> +		THERMAL_DEVICE_DISABLED);
> +	}
> +}
> +
> +static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
> +{
> +	struct rockchip_thermal_data *data = dev;
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +	int chn;
> +
> +	if (tsadc->irq_handle)
> +		tsadc->irq_handle(data->regs);
> +
> +	for (chn = CPU; chn < SENSOR_ID_END; chn++)
> +		thermal_zone_device_update(data->tz[chn]);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int rockchip_thermal_set_trips(void *zone, long low, long high)
> +{
> +	struct rockchip_thermal_data *data = zone;
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +	u32 val;
> +
> +	low = clamp_val(low, LONG_MIN, LONG_MAX);
> +	high = clamp_val(high, LONG_MIN, LONG_MAX);
> +
> +	/* channel0 is RESERVE, not need to set trips*/
> +	if (data->chn == RESERVE)
> +		return 0;
> +
> +	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));

I do not think you need to re-read the temperature: you are already
goivent lower and upper trip points for current temperature, you just
need to set alarm to the upper one.

> +	if (val == 0)
> +		return -EPROBE_DEFER;

What -EPROBE_DEFER means in this context?

> +
> +	/* Update alarm value to next higher trip point */
> +	if (data->alarm_temp == data->temp_passive && val <=
> +		tsadc->temp_to_code(data->temp_passive))
> +		high = data->hw_shut_temp;
> +
> +	if (data->alarm_temp >= data->temp_passive && val >
> +		tsadc->temp_to_code(data->temp_passive)) {
> +		high = data->temp_passive;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rockchip_thermal_get_temp(void *zone, long *out_temp)
> +{
> +	struct rockchip_thermal_data *data = zone;
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +	u32 val;
> +
> +	if (data->chn == RESERVE)
> +		return 0;
> +
> +	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));
> +
> +	if (val < tsadc->temp_to_code(data->hw_shut_temp))
> +		*out_temp = 120000;

Why is this needed? If this is really needed please comment the code.

> +	else
> +		*out_temp = tsadc->code_to_temp(val);
> +
> +	return 0;
> +}
> +
> +static int rockchip_configure_from_dt(struct device *dev,
> +				      struct device_node *np,
> +				      struct rockchip_thermal_data *data)
> +{
> +	int shut_temp, reset_mode;
> +
> +	if (of_property_read_u32(np, "hw-shut-temp", &shut_temp)) {
> +		dev_warn(dev, "Missing default shutdown temp property\n");
> +		data->hw_shut_temp = data->pdata->hw_shut_temp;

I am still not sure why we want it in DT. I envision the set up as
follows:

passive trip point - defined in DT
critical trip point - defined in DT, causes shutdown performed by
thermal core for us
hardware critical - hard coded (defined by the hardware, no point in
changing), shuts down or resets the box automatically by the hardware,
no kernel involved.

> +	} else {
> +		data->hw_shut_temp = shut_temp;
> +	}
> +
> +	if (of_property_read_u32(np, "tsadc-ht-reset-mode", &reset_mode)) {
> +		dev_warn(dev, "Missing default reset mode property\n");
> +		data->reset_mode = data->pdata->reset_mode;
> +	} else {
> +		data->reset_mode = reset_mode;
> +	}
> +
> +	data->temp_passive = data->pdata->temp_passive;
> +
> +	return 0;
> +}
> +
> +static int rockchip_thermal_probe(struct platform_device *pdev)
> +{
> +	struct rockchip_thermal_data *data;
> +	const struct rockchip_tsadc_platform_data *tsadc;
> +	const struct of_device_id *match;
> +
> +	struct cpumask clip_cpus;
> +	struct resource *res;
> +	struct device_node *np = pdev->dev.of_node;
> +
> +	int ret, err, chn;
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(struct rockchip_thermal_data),
> +			    GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	data->regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(data->regs)) {
> +		dev_err(&pdev->dev, "Could not get tsadc source, %p\n",
> +			data->regs);
> +		return PTR_ERR(data->regs);
> +	}
> +
> +	match = of_match_node(of_rockchip_thermal_match, np);
> +	if (!match)
> +		return -ENXIO;
> +	data->pdata = (const struct rockchip_tsadc_platform_data *)match->data;
> +	if (!data->pdata)
> +		return -EINVAL;
> +	tsadc = data->pdata;
> +
> +	data->clk = devm_clk_get(&pdev->dev, "tsadc");
> +	if (IS_ERR(data->clk)) {
> +		dev_err(&pdev->dev, "failed to get tsadc clock\n");
> +		return PTR_ERR(data->clk);
> +	}
> +
> +	data->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
> +	if (IS_ERR(data->pclk)) {
> +		dev_err(&pdev->dev, "failed to get tsadc pclk\n");
> +		return PTR_ERR(data->pclk);
> +	}
> +
> +	/**
> +	* Use a default of 10KHz for the converter clock.
> +	* This may become user-configurable in the future.
> +	* Need to judge the data->clk is Divided by xin32k.
> +	* Need to retry it if the pmic hasn't output the xin32k.
> +	*/
> +	if (clk_get_rate(data->clk) == 0)
> +		return -EPROBE_DEFER;

What would cause the clock to be present but report 0 rate? Are we sure
that it will start reporting different data after binding some more
drivers?

> +	ret = clk_set_rate(data->clk, 10000);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to set tsadc clk rate, %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(data->clk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to enable converter clock\n");
> +		goto err_clk;
> +	}
> +
> +	ret = clk_prepare_enable(data->pclk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to enable pclk\n");
> +		goto err_pclk;
> +	}
> +
> +	cpumask_set_cpu(0, &clip_cpus);
> +	data->cdev = of_cpufreq_cooling_register(np, &clip_cpus);
> +	if (IS_ERR(data->cdev)) {
> +		dev_err(&pdev->dev, "failed to register cpufreq cooling device\n");
> +		goto disable_clk;
> +	}
> +
> +	data->irq = platform_get_irq(pdev, 0);
> +	if (data->irq < 0) {
> +		dev_err(&pdev->dev, "no irq resource?\n");
> +		goto disable_clk;
> +	}
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, data->irq, NULL,
> +					&rockchip_thermal_alarm_irq_thread,
> +					IRQF_ONESHOT, "rockchip_thermal",
> +					data);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"failed to request tsadc irq: %d\n", ret);
> +		goto disable_clk;
> +	}
> +
> +	ret = rockchip_configure_from_dt(&pdev->dev, np, data);
> +	if (ret)
> +		dev_err(&pdev->dev, "Parsing device tree data error.\n");
> +
> +	/* The system three Teperture Sensors be registered */
> +	for (chn = RESERVE; chn < SENSOR_ID_END; chn++) {
> +		data->chn = chn;
> +
> +		data->tz[chn] = thermal_zone_of_sensor_register(
> +				&pdev->dev, data->chn,
> +				data, rockchip_thermal_get_temp,
> +				NULL,
> +				rockchip_thermal_set_trips);
> +		if (IS_ERR(data->tz[chn])) {
> +			err = PTR_ERR(data->tz[chn]);
> +			dev_err(&pdev->dev, "failed to register sensor: %d\n",
> +				err);
> +			chn--;
> +			goto unregister_tzs;
> +		}

Do we have to have all zones defined? Why?

> +
> +		platform_set_drvdata(pdev, data);
> +
> +		rockchip_thermal_initialize(data);
> +		rockchip_thermal_control(data, true);
> +	}
> +	return 0;
> +
> +unregister_tzs:
> +	for (; chn >= RESERVE; chn--)
> +		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
> +	cpufreq_cooling_unregister(data->cdev);
> +
> +disable_clk:
> +err_pclk:
> +	clk_disable_unprepare(data->pclk);
> +err_clk:
> +	clk_disable_unprepare(data->clk);
> +
> +	return ret;
> +}
> +
> +static int rockchip_thermal_remove(struct platform_device *pdev)
> +{
> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
> +	int chn;
> +
> +	rockchip_thermal_control(data, false);
> +
> +	for (; chn >= RESERVE; chn--)

You are not initializing chn, it will blow up.

> +		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
> +	cpufreq_cooling_unregister(data->cdev);
> +
> +	clk_disable_unprepare(data->clk);
> +	clk_disable_unprepare(data->pclk);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int rockchip_thermal_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
> +
> +	rockchip_thermal_control(data, false);
> +
> +	clk_disable_unprepare(data->clk);
> +	clk_disable_unprepare(data->pclk);

Why do we unpreparing in suspend instead of just disabling?

> +
> +	return 0;
> +}
> +
> +static int rockchip_thermal_resume(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(data->pclk);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(data->clk);
> +	if (ret)
> +		return ret;
> +
> +	rockchip_thermal_initialize(data);
> +	rockchip_thermal_control(data, true);
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(rockchip_thermal_pm_ops,
> +			 rockchip_thermal_suspend, rockchip_thermal_resume);
> +
> +static struct platform_driver rockchip_thermal_driver = {
> +	.driver = {
> +		   .name = "rockchip-thermal",
> +		   .owner = THIS_MODULE,
> +		   .pm = &rockchip_thermal_pm_ops,
> +		   .of_match_table = of_rockchip_thermal_match,
> +		   },
> +	.probe = rockchip_thermal_probe,
> +	.remove = rockchip_thermal_remove,
> +};
> +
> +module_platform_driver(rockchip_thermal_driver);
> +
> +MODULE_DESCRIPTION("ROCKCHIP THERMAL Driver");
> +MODULE_AUTHOR("Rockchip, Inc.");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:rockchip-thermal");
> -- 
> 1.9.1
> 
> 

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v8 1/5] thermal: rockchip: add driver for thermal
@ 2014-10-10 17:46     ` Dmitry Torokhov
  0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2014-10-10 17:46 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Caesar,

On Fri, Oct 10, 2014 at 06:19:37PM +0800, Caesar Wang wrote:
> Thermal is TS-ADC Controller module supports
> user-defined mode and automatic mode.
> 
> User-defined mode refers,TSADC all the control signals entirely by
> software writing to register for direct control.
> 
> Automaic mode refers to the module automatically poll TSADC output,
> and the results were checked.If you find that the temperature High
> in a period of time,an interrupt is generated to the processor
> down-measures taken;If the temperature over a period of time High,
> the resulting TSHUT gave CRU module,let it reset the entire chip,
> or via GPIO give PMIC.
> 
> Signed-off-by: zhaoyifeng <zyf@rock-chips.com>
> Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
> ---
>  drivers/thermal/Kconfig            |   9 +
>  drivers/thermal/Makefile           |   1 +
>  drivers/thermal/rockchip_thermal.c | 628 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 638 insertions(+)
>  create mode 100644 drivers/thermal/rockchip_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index f9a1386..24845ae 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -133,6 +133,15 @@ config SPEAR_THERMAL
>  	  Enable this to plug the SPEAr thermal sensor driver into the Linux
>  	  thermal framework.
>  
> +config ROCKCHIP_THERMAL
> +	tristate "Rockchip thermal driver"
> +	depends on ARCH_ROCKCHIP
> +	help
> +	  Rockchip thermal driver provides support for Temperature sensor
> +	  ADC (TS-ADC) found on Rockchip SoCs. It supports one critical
> +	  trip point. Cpufreq is used as the cooling device and will throttle
> +	  CPUs when the Temperature crosses the passive trip point.
> +
>  config RCAR_THERMAL
>  	tristate "Renesas R-Car thermal driver"
>  	depends on ARCH_SHMOBILE || COMPILE_TEST
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index de0636a..930554f 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -19,6 +19,7 @@ thermal_sys-$(CONFIG_CPU_THERMAL)	+= cpu_cooling.o
>  
>  # platform thermal drivers
>  obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
> +obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
>  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
>  obj-$(CONFIG_KIRKWOOD_THERMAL)  += kirkwood_thermal.o
>  obj-y				+= samsung/
> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> new file mode 100644
> index 0000000..ceee2c1
> --- /dev/null
> +++ b/drivers/thermal/rockchip_thermal.c
> @@ -0,0 +1,628 @@
> +/*
> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/cpu_cooling.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/thermal.h>
> +
> +/**
> +* If the temperature over a period of time High,
> +* the resulting TSHUT gave CRU module,let it reset the entire chip,
> +* or via GPIO give PMIC.
> +*/
> +enum reset_mde {

Can we spare one more letter? ;)

> +	CRU = 0,
> +	GPIO,
> +};
> +
> +/**
> +* The system has three Teperture Sensors.
> +* channel0 is reserve,channel1 is for CPU,and
> +* channel channel 2 is for GPU.
> +*/
> +enum sensor_id {
> +	RESERVE = 0,
> +	CPU,
> +	GPU,
> +	SENSOR_ID_END,
> +};
> +
> +struct rockchip_thermal_data {
> +	const struct rockchip_tsadc_platform_data *pdata;
> +	struct thermal_zone_device *tz[SENSOR_ID_END];
> +	struct thermal_cooling_device *cdev;
> +	void __iomem *regs;
> +
> +	unsigned long temp_passive;
> +	unsigned long hw_shut_temp;
> +	unsigned long alarm_temp;
> +	bool irq_enabled;
> +	int irq;
> +	int reset_mode;
> +	int chn;
> +
> +	struct clk *clk;
> +	struct clk *pclk;
> +};
> +
> +struct rockchip_tsadc_platform_data {
> +	unsigned long temp_passive;
> +	unsigned long hw_shut_temp;
> +	int reset_mode;
> +
> +	int (*irq_handle)(void __iomem *reg);

Why does it return int and not void? What is the return value of this?

> +	int (*initialize)(int reset_mode, int chn, void __iomem *reg,
Why does it return int and not void? What is the return value of this?> +			  unsigned long hw_shut_temp);
> +	int (*control)(void __iomem *reg, bool on);
> +	int (*code_to_temp)(u32 code);
> +	u32 (*temp_to_code)(int temp);
> +	void (*set_alarm_temp)(int chn, void __iomem *reg,
> +			       unsigned long alarm_temp);
> +};
> +
> +/* TSADC V2 Sensor info define: */
> +#define TSADCV2_AUTO_CON			0x04
> +#define TSADCV2_INT_EN				0x08
> +#define TSADCV2_INT_PD				0x0c
> +#define TSADCV2_DATA(chn)			(0x20+chn*0x04)

Unsafe macros. You want to enclose argument in parenthesis.

> +#define TSADCV2_COMP_INT(chn)		        (0x30+chn*0x04)
> +#define TSADCV2_COMP_SHUT(chn)		        (0x40+chn*0x04)
> +#define TSADCV2_HIGHT_INT_DEBOUNCE		0x60
> +#define TSADCV2_HIGHT_TSHUT_DEBOUNCE		0x64
> +#define TSADCV2_AUTO_PERIOD			0x68
> +#define TSADCV2_AUTO_PERIOD_HT			0x6c
> +
> +#define TSADCV2_AUTO_EN				BIT(0)
> +#define TSADCV2_AUTO_DISABLE			~BIT(0)
> +#define TSADCV2_AUTO_SRC_EN(chn)	        (0xf << (4 + chn))
> +#define TSADCV2_AUTO_TSHUT_POLARITY_HIGH	BIT(8)
> +
> +#define TSADCV2_INT_SRC_EN(chn)			BIT(chn)
> +#define TSADCV2_SHUT_2GPIO_SRC_EN(chn)		(0xf << (4 + chn))
> +#define TSADCV2_SHUT_2CRU_SRC_EN(chn)		(0xf << (8 + chn))
> +
> +#define TSADCV2_INT_PD_CLEAR			~BIT(8)
> +
> +#define TSADCV2_DATA_MASK			0xfff
> +#define TSADCV2_HIGHT_INT_DEBOUNCE_TIME		0x0a
> +#define TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME	0x0a
> +#define TSADCV2_AUTO_PERIOD_TIME		0x03e8
> +#define TSADCV2_AUTO_PERIOD_HT_TIME		0x64
> +
> +struct tsadc_table {
> +	unsigned long code;
> +	int temp;
> +};
> +
> +static const struct tsadc_table v2_code_table[] = {
> +	{TSADCV2_DATA_MASK, -40000},
> +	{3800, -40000},
> +	{3792, -35000},
> +	{3783, -30000},
> +	{3774, -25000},
> +	{3765, -20000},
> +	{3756, -15000},
> +	{3747, -10000},
> +	{3737, -5000},
> +	{3728, 0},
> +	{3718, 5000},
> +	{3708, 10000},
> +	{3698, 15000},
> +	{3688, 20000},
> +	{3678, 25000},
> +	{3667, 30000},
> +	{3656, 35000},
> +	{3645, 40000},
> +	{3634, 45000},
> +	{3623, 50000},
> +	{3611, 55000},
> +	{3600, 60000},
> +	{3588, 65000},
> +	{3575, 70000},
> +	{3563, 75000},
> +	{3550, 80000},
> +	{3537, 85000},
> +	{3524, 90000},
> +	{3510, 95000},
> +	{3496, 100000},
> +	{3482, 105000},
> +	{3467, 110000},
> +	{3452, 115000},
> +	{3437, 120000},
> +	{3421, 125000},
> +	{0, 125000},
> +};
> +
> +static int rk_tsadcv2_irq_handle(void __iomem *regs)
> +{
> +	u32 val;
> +
> +	val = readl_relaxed(regs + TSADCV2_INT_PD);
> +	writel_relaxed(val & TSADCV2_INT_PD_CLEAR, regs + TSADCV2_INT_PD);
> +
> +	return 0;
> +}
> +
> +static u32 rk_tsadcv2_temp_to_code(int temp)
> +{
> +	int high, low, mid, ret = 0;
> +
> +	low = 0;
> +	high = ARRAY_SIZE(v2_code_table) - 1;
> +	mid = (high + low) / 2;
> +
> +	if (temp < v2_code_table[low].temp || temp > v2_code_table[high].temp) {
> +		ret = -ERANGE;

The function returns u32, you can't return -ERANGE here.

> +		goto exit;
> +	}
> +
> +	while (low <= high) {
> +		if (temp == v2_code_table[mid].temp)
> +			return v2_code_table[mid].code;
> +		else if (temp < v2_code_table[mid].temp)
> +			high = mid - 1;
> +		else
> +			low = mid + 1;
> +		mid = (low + high) / 2;
> +	}

Hmm, you optimized temp_to_code but not code_to_temp, why?

> +
> +	return 0;
> +
> +exit:
> +	return ret;
> +}
> +
> +static int rk_tsadcv2_code_to_temp(u32 code)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(v2_code_table) - 1; i++) {
> +		if (code >= v2_code_table[i].code)
> +			return v2_code_table[i].temp;
> +	}
> +
> +	/* No code available,return max temperture */
> +	return 125000;
> +}
> +
> +static int rk_tsadcv2_initialize(int reset_mode, int chn, void __iomem *regs,
> +				 unsigned long hw_shut_temp)
> +{
> +	u32 shutdown_value;
> +
> +	shutdown_value = rk_tsadcv2_temp_to_code(hw_shut_temp);
> +
> +	/* Enable measurements at ~ 10 Hz */
> +	writel_relaxed(0 | TSADCV2_AUTO_TSHUT_POLARITY_HIGH, regs +
> +		       TSADCV2_AUTO_CON);
> +	writel_relaxed(TSADCV2_AUTO_PERIOD_TIME, regs + TSADCV2_AUTO_PERIOD);
> +	writel_relaxed(TSADCV2_AUTO_PERIOD_HT_TIME, regs +
> +		       TSADCV2_AUTO_PERIOD_HT);
> +	writel_relaxed(shutdown_value, regs + TSADCV2_COMP_SHUT(chn));
> +	writel_relaxed(TSADCV2_HIGHT_INT_DEBOUNCE_TIME, regs +
> +		       TSADCV2_HIGHT_INT_DEBOUNCE);
> +	writel_relaxed(TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME, regs +
> +		       TSADCV2_HIGHT_TSHUT_DEBOUNCE);
> +
> +	if (reset_mode == GPIO)
> +		writel_relaxed(TSADCV2_SHUT_2GPIO_SRC_EN(chn) |
> +			       TSADCV2_INT_SRC_EN(chn), regs +
> +			       TSADCV2_INT_EN);
> +	else
> +		writel_relaxed(TSADCV2_SHUT_2CRU_SRC_EN(chn) |
> +			       TSADCV2_INT_SRC_EN(chn) , regs +
> +			       TSADCV2_INT_EN);
> +
> +	writel_relaxed(TSADCV2_AUTO_SRC_EN(chn) | TSADCV2_AUTO_EN, regs +
> +		       TSADCV2_AUTO_CON);
> +
> +	return 0;
> +}
> +
> +static int rk_tsadcv2_control(void __iomem *regs, bool on)
> +{
> +	u32 val;
> +
> +	if (on) {
> +		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
> +		writel_relaxed(val | TSADCV2_AUTO_EN, regs + TSADCV2_AUTO_CON);
> +	} else {
> +		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
> +		writel_relaxed(val & TSADCV2_AUTO_DISABLE,
> +			       regs + TSADCV2_AUTO_CON);
> +	}
> +
> +	return 0;
> +}
> +
> +static void rk_tsadcv2_alarm_temp(int chn, void __iomem *regs,
> +				  unsigned long alarm_temp)
> +{
> +	u32 alarm_value;
> +
> +	alarm_value = rk_tsadcv2_temp_to_code(alarm_temp);
> +
> +	writel_relaxed(alarm_value & TSADCV2_DATA_MASK, regs +
> +		       TSADCV2_COMP_INT(chn));
> +}
> +
> +static const struct rockchip_tsadc_platform_data rk3288_tsadc_data = {
> +	.reset_mode = GPIO, /* default TSHUT via GPIO give PMIC */
> +	.temp_passive = 80000,
> +	.hw_shut_temp = 120000,
> +	.irq_handle = rk_tsadcv2_irq_handle,
> +	.initialize = rk_tsadcv2_initialize,
> +	.control = rk_tsadcv2_control,
> +	.code_to_temp = rk_tsadcv2_code_to_temp,
> +	.temp_to_code = rk_tsadcv2_temp_to_code,
> +	.set_alarm_temp = rk_tsadcv2_alarm_temp,
> +};
> +
> +static const struct of_device_id of_rockchip_thermal_match[] = {
> +	{
> +		.compatible = "rockchip,rk3288-tsadc",
> +		.data = (void *)&rk3288_tsadc_data,
> +	},
> +	{ /* end */ },
> +};
> +MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
> +
> +static void rockchip_set_alarm_temp(struct rockchip_thermal_data *data,
> +				    int alarm_temp)
> +{
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +
> +	data->alarm_temp = alarm_temp;
> +	if (tsadc->set_alarm_temp)
> +		tsadc->set_alarm_temp(data->chn, data->regs, alarm_temp);
> +}
> +
> +static int rockchip_thermal_initialize(struct rockchip_thermal_data *data)
> +{
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +
> +	if (data->chn == RESERVE)
> +		return 0;
> +
> +	if (tsadc->initialize)
> +		tsadc->initialize(tsadc->reset_mode, data->chn, data->regs,
> +				  data->hw_shut_temp);
> +
> +	rockchip_set_alarm_temp(data, data->temp_passive);
> +
> +	return 0;
> +}
> +
> +static void rockchip_thermal_control(struct rockchip_thermal_data *data,
> +				     bool on)
> +{
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +
> +	if (tsadc->control)
> +		tsadc->control(data->regs, on);
> +
> +	if (on) {
> +		data->irq_enabled = true;
> +		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
> +		THERMAL_DEVICE_ENABLED);
> +	} else {
> +		data->irq_enabled = false;
> +		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
> +		THERMAL_DEVICE_DISABLED);
> +	}
> +}
> +
> +static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
> +{
> +	struct rockchip_thermal_data *data = dev;
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +	int chn;
> +
> +	if (tsadc->irq_handle)
> +		tsadc->irq_handle(data->regs);
> +
> +	for (chn = CPU; chn < SENSOR_ID_END; chn++)
> +		thermal_zone_device_update(data->tz[chn]);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int rockchip_thermal_set_trips(void *zone, long low, long high)
> +{
> +	struct rockchip_thermal_data *data = zone;
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +	u32 val;
> +
> +	low = clamp_val(low, LONG_MIN, LONG_MAX);
> +	high = clamp_val(high, LONG_MIN, LONG_MAX);
> +
> +	/* channel0 is RESERVE, not need to set trips*/
> +	if (data->chn == RESERVE)
> +		return 0;
> +
> +	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));

I do not think you need to re-read the temperature: you are already
goivent lower and upper trip points for current temperature, you just
need to set alarm to the upper one.

> +	if (val == 0)
> +		return -EPROBE_DEFER;

What -EPROBE_DEFER means in this context?

> +
> +	/* Update alarm value to next higher trip point */
> +	if (data->alarm_temp == data->temp_passive && val <=
> +		tsadc->temp_to_code(data->temp_passive))
> +		high = data->hw_shut_temp;
> +
> +	if (data->alarm_temp >= data->temp_passive && val >
> +		tsadc->temp_to_code(data->temp_passive)) {
> +		high = data->temp_passive;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rockchip_thermal_get_temp(void *zone, long *out_temp)
> +{
> +	struct rockchip_thermal_data *data = zone;
> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
> +	u32 val;
> +
> +	if (data->chn == RESERVE)
> +		return 0;
> +
> +	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));
> +
> +	if (val < tsadc->temp_to_code(data->hw_shut_temp))
> +		*out_temp = 120000;

Why is this needed? If this is really needed please comment the code.

> +	else
> +		*out_temp = tsadc->code_to_temp(val);
> +
> +	return 0;
> +}
> +
> +static int rockchip_configure_from_dt(struct device *dev,
> +				      struct device_node *np,
> +				      struct rockchip_thermal_data *data)
> +{
> +	int shut_temp, reset_mode;
> +
> +	if (of_property_read_u32(np, "hw-shut-temp", &shut_temp)) {
> +		dev_warn(dev, "Missing default shutdown temp property\n");
> +		data->hw_shut_temp = data->pdata->hw_shut_temp;

I am still not sure why we want it in DT. I envision the set up as
follows:

passive trip point - defined in DT
critical trip point - defined in DT, causes shutdown performed by
thermal core for us
hardware critical - hard coded (defined by the hardware, no point in
changing), shuts down or resets the box automatically by the hardware,
no kernel involved.

> +	} else {
> +		data->hw_shut_temp = shut_temp;
> +	}
> +
> +	if (of_property_read_u32(np, "tsadc-ht-reset-mode", &reset_mode)) {
> +		dev_warn(dev, "Missing default reset mode property\n");
> +		data->reset_mode = data->pdata->reset_mode;
> +	} else {
> +		data->reset_mode = reset_mode;
> +	}
> +
> +	data->temp_passive = data->pdata->temp_passive;
> +
> +	return 0;
> +}
> +
> +static int rockchip_thermal_probe(struct platform_device *pdev)
> +{
> +	struct rockchip_thermal_data *data;
> +	const struct rockchip_tsadc_platform_data *tsadc;
> +	const struct of_device_id *match;
> +
> +	struct cpumask clip_cpus;
> +	struct resource *res;
> +	struct device_node *np = pdev->dev.of_node;
> +
> +	int ret, err, chn;
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(struct rockchip_thermal_data),
> +			    GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	data->regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(data->regs)) {
> +		dev_err(&pdev->dev, "Could not get tsadc source, %p\n",
> +			data->regs);
> +		return PTR_ERR(data->regs);
> +	}
> +
> +	match = of_match_node(of_rockchip_thermal_match, np);
> +	if (!match)
> +		return -ENXIO;
> +	data->pdata = (const struct rockchip_tsadc_platform_data *)match->data;
> +	if (!data->pdata)
> +		return -EINVAL;
> +	tsadc = data->pdata;
> +
> +	data->clk = devm_clk_get(&pdev->dev, "tsadc");
> +	if (IS_ERR(data->clk)) {
> +		dev_err(&pdev->dev, "failed to get tsadc clock\n");
> +		return PTR_ERR(data->clk);
> +	}
> +
> +	data->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
> +	if (IS_ERR(data->pclk)) {
> +		dev_err(&pdev->dev, "failed to get tsadc pclk\n");
> +		return PTR_ERR(data->pclk);
> +	}
> +
> +	/**
> +	* Use a default of 10KHz for the converter clock.
> +	* This may become user-configurable in the future.
> +	* Need to judge the data->clk is Divided by xin32k.
> +	* Need to retry it if the pmic hasn't output the xin32k.
> +	*/
> +	if (clk_get_rate(data->clk) == 0)
> +		return -EPROBE_DEFER;

What would cause the clock to be present but report 0 rate? Are we sure
that it will start reporting different data after binding some more
drivers?

> +	ret = clk_set_rate(data->clk, 10000);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to set tsadc clk rate, %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(data->clk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to enable converter clock\n");
> +		goto err_clk;
> +	}
> +
> +	ret = clk_prepare_enable(data->pclk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to enable pclk\n");
> +		goto err_pclk;
> +	}
> +
> +	cpumask_set_cpu(0, &clip_cpus);
> +	data->cdev = of_cpufreq_cooling_register(np, &clip_cpus);
> +	if (IS_ERR(data->cdev)) {
> +		dev_err(&pdev->dev, "failed to register cpufreq cooling device\n");
> +		goto disable_clk;
> +	}
> +
> +	data->irq = platform_get_irq(pdev, 0);
> +	if (data->irq < 0) {
> +		dev_err(&pdev->dev, "no irq resource?\n");
> +		goto disable_clk;
> +	}
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, data->irq, NULL,
> +					&rockchip_thermal_alarm_irq_thread,
> +					IRQF_ONESHOT, "rockchip_thermal",
> +					data);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"failed to request tsadc irq: %d\n", ret);
> +		goto disable_clk;
> +	}
> +
> +	ret = rockchip_configure_from_dt(&pdev->dev, np, data);
> +	if (ret)
> +		dev_err(&pdev->dev, "Parsing device tree data error.\n");
> +
> +	/* The system three Teperture Sensors be registered */
> +	for (chn = RESERVE; chn < SENSOR_ID_END; chn++) {
> +		data->chn = chn;
> +
> +		data->tz[chn] = thermal_zone_of_sensor_register(
> +				&pdev->dev, data->chn,
> +				data, rockchip_thermal_get_temp,
> +				NULL,
> +				rockchip_thermal_set_trips);
> +		if (IS_ERR(data->tz[chn])) {
> +			err = PTR_ERR(data->tz[chn]);
> +			dev_err(&pdev->dev, "failed to register sensor: %d\n",
> +				err);
> +			chn--;
> +			goto unregister_tzs;
> +		}

Do we have to have all zones defined? Why?

> +
> +		platform_set_drvdata(pdev, data);
> +
> +		rockchip_thermal_initialize(data);
> +		rockchip_thermal_control(data, true);
> +	}
> +	return 0;
> +
> +unregister_tzs:
> +	for (; chn >= RESERVE; chn--)
> +		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
> +	cpufreq_cooling_unregister(data->cdev);
> +
> +disable_clk:
> +err_pclk:
> +	clk_disable_unprepare(data->pclk);
> +err_clk:
> +	clk_disable_unprepare(data->clk);
> +
> +	return ret;
> +}
> +
> +static int rockchip_thermal_remove(struct platform_device *pdev)
> +{
> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
> +	int chn;
> +
> +	rockchip_thermal_control(data, false);
> +
> +	for (; chn >= RESERVE; chn--)

You are not initializing chn, it will blow up.

> +		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
> +	cpufreq_cooling_unregister(data->cdev);
> +
> +	clk_disable_unprepare(data->clk);
> +	clk_disable_unprepare(data->pclk);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int rockchip_thermal_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
> +
> +	rockchip_thermal_control(data, false);
> +
> +	clk_disable_unprepare(data->clk);
> +	clk_disable_unprepare(data->pclk);

Why do we unpreparing in suspend instead of just disabling?

> +
> +	return 0;
> +}
> +
> +static int rockchip_thermal_resume(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(data->pclk);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(data->clk);
> +	if (ret)
> +		return ret;
> +
> +	rockchip_thermal_initialize(data);
> +	rockchip_thermal_control(data, true);
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(rockchip_thermal_pm_ops,
> +			 rockchip_thermal_suspend, rockchip_thermal_resume);
> +
> +static struct platform_driver rockchip_thermal_driver = {
> +	.driver = {
> +		   .name = "rockchip-thermal",
> +		   .owner = THIS_MODULE,
> +		   .pm = &rockchip_thermal_pm_ops,
> +		   .of_match_table = of_rockchip_thermal_match,
> +		   },
> +	.probe = rockchip_thermal_probe,
> +	.remove = rockchip_thermal_remove,
> +};
> +
> +module_platform_driver(rockchip_thermal_driver);
> +
> +MODULE_DESCRIPTION("ROCKCHIP THERMAL Driver");
> +MODULE_AUTHOR("Rockchip, Inc.");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:rockchip-thermal");
> -- 
> 1.9.1
> 
> 

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v8 1/5] thermal: rockchip: add driver for thermal
  2014-10-10 17:46     ` Dmitry Torokhov
@ 2014-10-11  6:01       ` Caesar Wang
  -1 siblings, 0 replies; 14+ messages in thread
From: Caesar Wang @ 2014-10-11  6:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: heiko, rui.zhang, edubezval, arnd, zyf, dianders, linux-rockchip,
	linux-kernel, linux-pm, linux-arm-kernel, devicetree, linux-doc,
	cf, dbasehore, huangtao, cjf, zhengsq

Dear Dmitry,

在 2014年10月11日 01:46, Dmitry Torokhov 写道:
> Hi Caesar,
>
> On Fri, Oct 10, 2014 at 06:19:37PM +0800, Caesar Wang wrote:
>> Thermal is TS-ADC Controller module supports
>> user-defined mode and automatic mode.
>>
>> User-defined mode refers,TSADC all the control signals entirely by
>> software writing to register for direct control.
>>
>> Automaic mode refers to the module automatically poll TSADC output,
>> and the results were checked.If you find that the temperature High
>> in a period of time,an interrupt is generated to the processor
>> down-measures taken;If the temperature over a period of time High,
>> the resulting TSHUT gave CRU module,let it reset the entire chip,
>> or via GPIO give PMIC.
>>
>> Signed-off-by: zhaoyifeng <zyf@rock-chips.com>
>> Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
>> ---
>>   drivers/thermal/Kconfig            |   9 +
>>   drivers/thermal/Makefile           |   1 +
>>   drivers/thermal/rockchip_thermal.c | 628 +++++++++++++++++++++++++++++++++++++
>>   3 files changed, 638 insertions(+)
>>   create mode 100644 drivers/thermal/rockchip_thermal.c
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index f9a1386..24845ae 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -133,6 +133,15 @@ config SPEAR_THERMAL
>>   	  Enable this to plug the SPEAr thermal sensor driver into the Linux
>>   	  thermal framework.
>>   
>> +config ROCKCHIP_THERMAL
>> +	tristate "Rockchip thermal driver"
>> +	depends on ARCH_ROCKCHIP
>> +	help
>> +	  Rockchip thermal driver provides support for Temperature sensor
>> +	  ADC (TS-ADC) found on Rockchip SoCs. It supports one critical
>> +	  trip point. Cpufreq is used as the cooling device and will throttle
>> +	  CPUs when the Temperature crosses the passive trip point.
>> +
>>   config RCAR_THERMAL
>>   	tristate "Renesas R-Car thermal driver"
>>   	depends on ARCH_SHMOBILE || COMPILE_TEST
>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>> index de0636a..930554f 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -19,6 +19,7 @@ thermal_sys-$(CONFIG_CPU_THERMAL)	+= cpu_cooling.o
>>   
>>   # platform thermal drivers
>>   obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
>> +obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
>>   obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
>>   obj-$(CONFIG_KIRKWOOD_THERMAL)  += kirkwood_thermal.o
>>   obj-y				+= samsung/
>> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
>> new file mode 100644
>> index 0000000..ceee2c1
>> --- /dev/null
>> +++ b/drivers/thermal/rockchip_thermal.c
>> @@ -0,0 +1,628 @@
>> +/*
>> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/cpu_cooling.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/thermal.h>
>> +
>> +/**
>> +* If the temperature over a period of time High,
>> +* the resulting TSHUT gave CRU module,let it reset the entire chip,
>> +* or via GPIO give PMIC.
>> +*/
>> +enum reset_mde {
> Can we spare one more letter? ;)
>

OK, maybe fix it as the follows:

enum reset_mde {
     TSADC_HT_RESET_CRU = 0,
     TSADC_HT_PULL_GPIO,
};
>> +	CRU = 0,
>> +	GPIO,
>> +};
>> +
>> +/**
>> +* The system has three Teperture Sensors.
>> +* channel0 is reserve,channel1 is for CPU,and
>> +* channel channel 2 is for GPU.
>> +*/
>> +enum sensor_id {
>> +	RESERVE = 0,
>> +	CPU,
>> +	GPU,
>> +	SENSOR_ID_END,
>> +};
>> +
>> +struct rockchip_thermal_data {
>> +	const struct rockchip_tsadc_platform_data *pdata;
>> +	struct thermal_zone_device *tz[SENSOR_ID_END];
>> +	struct thermal_cooling_device *cdev;
>> +	void __iomem *regs;
>> +
>> +	unsigned long temp_passive;
>> +	unsigned long hw_shut_temp;
>> +	unsigned long alarm_temp;
>> +	bool irq_enabled;
>> +	int irq;
>> +	int reset_mode;
>> +	int chn;
>> +
>> +	struct clk *clk;
>> +	struct clk *pclk;
>> +};
>> +
>> +struct rockchip_tsadc_platform_data {
>> +	unsigned long temp_passive;
>> +	unsigned long hw_shut_temp;
>> +	int reset_mode;
>> +
>> +	int (*irq_handle)(void __iomem *reg);
> Why does it return int and not void? What is the return value of this?

Fixed. instead of using void.
>> +	int (*initialize)(int reset_mode, int chn, void __iomem *reg,
> Why does it return int and not void? What is the return value of this?> +			  unsigned long hw_shut_temp);

Ditto.

>> +	int (*control)(void __iomem *reg, bool on);
>> +	int (*code_to_temp)(u32 code);
>> +	u32 (*temp_to_code)(int temp);
>> +	void (*set_alarm_temp)(int chn, void __iomem *reg,
>> +			       unsigned long alarm_temp);
>> +};
>> +
>> +/* TSADC V2 Sensor info define: */
>> +#define TSADCV2_AUTO_CON			0x04
>> +#define TSADCV2_INT_EN				0x08
>> +#define TSADCV2_INT_PD				0x0c
>> +#define TSADCV2_DATA(chn)			(0x20+chn*0x04)
> Unsafe macros. You want to enclose argument in parenthesis.

Yeah.
Hmmm, there is no thought of a good way to deal with it at the moment.

Maybe do that as the follows,but should be a better way to deal.

e.g:
#define TSADCV2_DATA0  0x20
#define TSADCV2_DATA1  0x24
#define TSADCV2_DATA2  0x28
#define TSADCV2_DATA3  0x2c

#define TSADCV2_COMP0_INT  0x30
#define TSADCV2_COMP1_INT  0x34
#define TSADCV2_COMP2_INT  0x38
#define TSADCV2_COMP3_INT  0x3c

#define TSADCV2_COMP0_SHUT  0x40
#define TSADCV2_COMP1_SHUT  0x44
#define TSADCV2_COMP2_SHUT  0x48
#define TSADCV2_COMP3_SHUT  0x4c
>> +#define TSADCV2_COMP_INT(chn)		        (0x30+chn*0x04)
>> +#define TSADCV2_COMP_SHUT(chn)		        (0x40+chn*0x04)
>> +#define TSADCV2_HIGHT_INT_DEBOUNCE		0x60
>> +#define TSADCV2_HIGHT_TSHUT_DEBOUNCE		0x64
>> +#define TSADCV2_AUTO_PERIOD			0x68
>> +#define TSADCV2_AUTO_PERIOD_HT			0x6c
>> +
>> +#define TSADCV2_AUTO_EN				BIT(0)
>> +#define TSADCV2_AUTO_DISABLE			~BIT(0)
>> +#define TSADCV2_AUTO_SRC_EN(chn)	        (0xf << (4 + chn))
>> +#define TSADCV2_AUTO_TSHUT_POLARITY_HIGH	BIT(8)
>> +
>> +#define TSADCV2_INT_SRC_EN(chn)			BIT(chn)
>> +#define TSADCV2_SHUT_2GPIO_SRC_EN(chn)		(0xf << (4 + chn))
>> +#define TSADCV2_SHUT_2CRU_SRC_EN(chn)		(0xf << (8 + chn))
>> +
>> +#define TSADCV2_INT_PD_CLEAR			~BIT(8)
>> +
>> +#define TSADCV2_DATA_MASK			0xfff
>> +#define TSADCV2_HIGHT_INT_DEBOUNCE_TIME		0x0a
>> +#define TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME	0x0a
>> +#define TSADCV2_AUTO_PERIOD_TIME		0x03e8
>> +#define TSADCV2_AUTO_PERIOD_HT_TIME		0x64
>> +
>> +struct tsadc_table {
>> +	unsigned long code;
>> +	int temp;
>> +};
>> +
>> +static const struct tsadc_table v2_code_table[] = {
>> +	{TSADCV2_DATA_MASK, -40000},
>> +	{3800, -40000},
>> +	{3792, -35000},
>> +	{3783, -30000},
>> +	{3774, -25000},
>> +	{3765, -20000},
>> +	{3756, -15000},
>> +	{3747, -10000},
>> +	{3737, -5000},
>> +	{3728, 0},
>> +	{3718, 5000},
>> +	{3708, 10000},
>> +	{3698, 15000},
>> +	{3688, 20000},
>> +	{3678, 25000},
>> +	{3667, 30000},
>> +	{3656, 35000},
>> +	{3645, 40000},
>> +	{3634, 45000},
>> +	{3623, 50000},
>> +	{3611, 55000},
>> +	{3600, 60000},
>> +	{3588, 65000},
>> +	{3575, 70000},
>> +	{3563, 75000},
>> +	{3550, 80000},
>> +	{3537, 85000},
>> +	{3524, 90000},
>> +	{3510, 95000},
>> +	{3496, 100000},
>> +	{3482, 105000},
>> +	{3467, 110000},
>> +	{3452, 115000},
>> +	{3437, 120000},
>> +	{3421, 125000},
>> +	{0, 125000},
>> +};
>> +
>> +static int rk_tsadcv2_irq_handle(void __iomem *regs)
>> +{
>> +	u32 val;
>> +
>> +	val = readl_relaxed(regs + TSADCV2_INT_PD);
>> +	writel_relaxed(val & TSADCV2_INT_PD_CLEAR, regs + TSADCV2_INT_PD);
>> +
>> +	return 0;
>> +}
>> +
>> +static u32 rk_tsadcv2_temp_to_code(int temp)
>> +{
>> +	int high, low, mid, ret = 0;
>> +
>> +	low = 0;
>> +	high = ARRAY_SIZE(v2_code_table) - 1;
>> +	mid = (high + low) / 2;
>> +
>> +	if (temp < v2_code_table[low].temp || temp > v2_code_table[high].temp) {
>> +		ret = -ERANGE;
> The function returns u32, you can't return -ERANGE here.

Yeah,fixed.
>> +		goto exit;
>> +	}
>> +
>> +	while (low <= high) {
>> +		if (temp == v2_code_table[mid].temp)
>> +			return v2_code_table[mid].code;
>> +		else if (temp < v2_code_table[mid].temp)
>> +			high = mid - 1;
>> +		else
>> +			low = mid + 1;
>> +		mid = (low + high) / 2;
>> +	}
> Hmm, you optimized temp_to_code but not code_to_temp, why?

OK,fixed.
>> +
>> +	return 0;
>> +
>> +exit:
>> +	return ret;
>> +}
>> +
>> +static int rk_tsadcv2_code_to_temp(u32 code)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(v2_code_table) - 1; i++) {
>> +		if (code >= v2_code_table[i].code)
>> +			return v2_code_table[i].temp;
>> +	}
>> +
>> +	/* No code available,return max temperture */
>> +	return 125000;
>> +}
>> +
>> +static int rk_tsadcv2_initialize(int reset_mode, int chn, void __iomem *regs,
>> +				 unsigned long hw_shut_temp)
>> +{
>> +	u32 shutdown_value;
>> +
>> +	shutdown_value = rk_tsadcv2_temp_to_code(hw_shut_temp);
>> +
>> +	/* Enable measurements at ~ 10 Hz */
>> +	writel_relaxed(0 | TSADCV2_AUTO_TSHUT_POLARITY_HIGH, regs +
>> +		       TSADCV2_AUTO_CON);
>> +	writel_relaxed(TSADCV2_AUTO_PERIOD_TIME, regs + TSADCV2_AUTO_PERIOD);
>> +	writel_relaxed(TSADCV2_AUTO_PERIOD_HT_TIME, regs +
>> +		       TSADCV2_AUTO_PERIOD_HT);
>> +	writel_relaxed(shutdown_value, regs + TSADCV2_COMP_SHUT(chn));
>> +	writel_relaxed(TSADCV2_HIGHT_INT_DEBOUNCE_TIME, regs +
>> +		       TSADCV2_HIGHT_INT_DEBOUNCE);
>> +	writel_relaxed(TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME, regs +
>> +		       TSADCV2_HIGHT_TSHUT_DEBOUNCE);
>> +
>> +	if (reset_mode == GPIO)
>> +		writel_relaxed(TSADCV2_SHUT_2GPIO_SRC_EN(chn) |
>> +			       TSADCV2_INT_SRC_EN(chn), regs +
>> +			       TSADCV2_INT_EN);
>> +	else
>> +		writel_relaxed(TSADCV2_SHUT_2CRU_SRC_EN(chn) |
>> +			       TSADCV2_INT_SRC_EN(chn) , regs +
>> +			       TSADCV2_INT_EN);
>> +
>> +	writel_relaxed(TSADCV2_AUTO_SRC_EN(chn) | TSADCV2_AUTO_EN, regs +
>> +		       TSADCV2_AUTO_CON);
>> +
>> +	return 0;
>> +}
>> +
>> +static int rk_tsadcv2_control(void __iomem *regs, bool on)
>> +{
>> +	u32 val;
>> +
>> +	if (on) {
>> +		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
>> +		writel_relaxed(val | TSADCV2_AUTO_EN, regs + TSADCV2_AUTO_CON);
>> +	} else {
>> +		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
>> +		writel_relaxed(val & TSADCV2_AUTO_DISABLE,
>> +			       regs + TSADCV2_AUTO_CON);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void rk_tsadcv2_alarm_temp(int chn, void __iomem *regs,
>> +				  unsigned long alarm_temp)
>> +{
>> +	u32 alarm_value;
>> +
>> +	alarm_value = rk_tsadcv2_temp_to_code(alarm_temp);
>> +
>> +	writel_relaxed(alarm_value & TSADCV2_DATA_MASK, regs +
>> +		       TSADCV2_COMP_INT(chn));
>> +}
>> +
>> +static const struct rockchip_tsadc_platform_data rk3288_tsadc_data = {
>> +	.reset_mode = GPIO, /* default TSHUT via GPIO give PMIC */
>> +	.temp_passive = 80000,
>> +	.hw_shut_temp = 120000,
>> +	.irq_handle = rk_tsadcv2_irq_handle,
>> +	.initialize = rk_tsadcv2_initialize,
>> +	.control = rk_tsadcv2_control,
>> +	.code_to_temp = rk_tsadcv2_code_to_temp,
>> +	.temp_to_code = rk_tsadcv2_temp_to_code,
>> +	.set_alarm_temp = rk_tsadcv2_alarm_temp,
>> +};
>> +
>> +static const struct of_device_id of_rockchip_thermal_match[] = {
>> +	{
>> +		.compatible = "rockchip,rk3288-tsadc",
>> +		.data = (void *)&rk3288_tsadc_data,
>> +	},
>> +	{ /* end */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
>> +
>> +static void rockchip_set_alarm_temp(struct rockchip_thermal_data *data,
>> +				    int alarm_temp)
>> +{
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +
>> +	data->alarm_temp = alarm_temp;
>> +	if (tsadc->set_alarm_temp)
>> +		tsadc->set_alarm_temp(data->chn, data->regs, alarm_temp);
>> +}
>> +
>> +static int rockchip_thermal_initialize(struct rockchip_thermal_data *data)
>> +{
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +
>> +	if (data->chn == RESERVE)
>> +		return 0;
>> +
>> +	if (tsadc->initialize)
>> +		tsadc->initialize(tsadc->reset_mode, data->chn, data->regs,
>> +				  data->hw_shut_temp);
>> +
>> +	rockchip_set_alarm_temp(data, data->temp_passive);
>> +
>> +	return 0;
>> +}
>> +
>> +static void rockchip_thermal_control(struct rockchip_thermal_data *data,
>> +				     bool on)
>> +{
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +
>> +	if (tsadc->control)
>> +		tsadc->control(data->regs, on);
>> +
>> +	if (on) {
>> +		data->irq_enabled = true;
>> +		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
>> +		THERMAL_DEVICE_ENABLED);
>> +	} else {
>> +		data->irq_enabled = false;
>> +		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
>> +		THERMAL_DEVICE_DISABLED);
>> +	}
>> +}
>> +
>> +static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
>> +{
>> +	struct rockchip_thermal_data *data = dev;
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +	int chn;
>> +
>> +	if (tsadc->irq_handle)
>> +		tsadc->irq_handle(data->regs);
>> +
>> +	for (chn = CPU; chn < SENSOR_ID_END; chn++)
>> +		thermal_zone_device_update(data->tz[chn]);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int rockchip_thermal_set_trips(void *zone, long low, long high)
>> +{
>> +	struct rockchip_thermal_data *data = zone;
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +	u32 val;
>> +
>> +	low = clamp_val(low, LONG_MIN, LONG_MAX);
>> +	high = clamp_val(high, LONG_MIN, LONG_MAX);
>> +
>> +	/* channel0 is RESERVE, not need to set trips*/
>> +	if (data->chn == RESERVE)
>> +		return 0;
>> +
>> +	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));
> I do not think you need to re-read the temperature: you are already
> goivent lower and upper trip points for current temperature, you just
> need to set alarm to the upper one.
>

Fixed.
>> +	if (val == 0)
>> +		return -EPROBE_DEFER;
> What -EPROBE_DEFER means in this context?
>

In general, the A/D value of the channel last conversion need some time.
There isn't a need in here.
>> +
>> +	/* Update alarm value to next higher trip point */
>> +	if (data->alarm_temp == data->temp_passive && val <=
>> +		tsadc->temp_to_code(data->temp_passive))
>> +		high = data->hw_shut_temp;
>> +
>> +	if (data->alarm_temp >= data->temp_passive && val >
>> +		tsadc->temp_to_code(data->temp_passive)) {
>> +		high = data->temp_passive;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_thermal_get_temp(void *zone, long *out_temp)
>> +{
>> +	struct rockchip_thermal_data *data = zone;
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +	u32 val;
>> +
>> +	if (data->chn == RESERVE)
>> +		return 0;
>> +
>> +	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));
>> +
>> +	if (val < tsadc->temp_to_code(data->hw_shut_temp))
>> +		*out_temp = 120000;
> Why is this needed? If this is really needed please comment the code.

Fixed.
It's not need.

>> +	else
>> +		*out_temp = tsadc->code_to_temp(val);
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_configure_from_dt(struct device *dev,
>> +				      struct device_node *np,
>> +				      struct rockchip_thermal_data *data)
>> +{
>> +	int shut_temp, reset_mode;
>> +
>> +	if (of_property_read_u32(np, "hw-shut-temp", &shut_temp)) {
>> +		dev_warn(dev, "Missing default shutdown temp property\n");
>> +		data->hw_shut_temp = data->pdata->hw_shut_temp;
> I am still not sure why we want it in DT. I envision the set up as
> follows:
>
> passive trip point - defined in DT
> critical trip point - defined in DT, causes shutdown performed by
> thermal core for us
> hardware critical - hard coded (defined by the hardware, no point in
> changing), shuts down or resets the box automatically by the hardware,
> no kernel involved.

Although TSHUT(hardware shutdown temperature value) is not a generic 
trip point,
TSHUT value is relative fixed and need be init as the tsadc_comp.

hmm..,if  it doeen't define in the DT,we had to get a TSHUT  from data,

as the about:
dev_warn(dev, "Missing default shutdown temp property\n");
		data->hw_shut_temp = data->pdata->hw_shut_temp;



>> +	} else {
>> +		data->hw_shut_temp = shut_temp;
>> +	}
>> +
>> +	if (of_property_read_u32(np, "tsadc-ht-reset-mode", &reset_mode)) {
>> +		dev_warn(dev, "Missing default reset mode property\n");
>> +		data->reset_mode = data->pdata->reset_mode;
>> +	} else {
>> +		data->reset_mode = reset_mode;
>> +	}
>> +
>> +	data->temp_passive = data->pdata->temp_passive;
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_thermal_probe(struct platform_device *pdev)
>> +{
>> +	struct rockchip_thermal_data *data;
>> +	const struct rockchip_tsadc_platform_data *tsadc;
>> +	const struct of_device_id *match;
>> +
>> +	struct cpumask clip_cpus;
>> +	struct resource *res;
>> +	struct device_node *np = pdev->dev.of_node;
>> +
>> +	int ret, err, chn;
>> +
>> +	data = devm_kzalloc(&pdev->dev, sizeof(struct rockchip_thermal_data),
>> +			    GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	data->regs = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(data->regs)) {
>> +		dev_err(&pdev->dev, "Could not get tsadc source, %p\n",
>> +			data->regs);
>> +		return PTR_ERR(data->regs);
>> +	}
>> +
>> +	match = of_match_node(of_rockchip_thermal_match, np);
>> +	if (!match)
>> +		return -ENXIO;
>> +	data->pdata = (const struct rockchip_tsadc_platform_data *)match->data;
>> +	if (!data->pdata)
>> +		return -EINVAL;
>> +	tsadc = data->pdata;
>> +
>> +	data->clk = devm_clk_get(&pdev->dev, "tsadc");
>> +	if (IS_ERR(data->clk)) {
>> +		dev_err(&pdev->dev, "failed to get tsadc clock\n");
>> +		return PTR_ERR(data->clk);
>> +	}
>> +
>> +	data->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
>> +	if (IS_ERR(data->pclk)) {
>> +		dev_err(&pdev->dev, "failed to get tsadc pclk\n");
>> +		return PTR_ERR(data->pclk);
>> +	}
>> +
>> +	/**
>> +	* Use a default of 10KHz for the converter clock.
>> +	* This may become user-configurable in the future.
>> +	* Need to judge the data->clk is Divided by xin32k.
>> +	* Need to retry it if the pmic hasn't output the xin32k.
>> +	*/
>> +	if (clk_get_rate(data->clk) == 0)
>> +		return -EPROBE_DEFER;
> What would cause the clock to be present but report 0 rate? Are we sure
> that it will start reporting different data after binding some more
> drivers?
>

The clk is divided by xin32k from PMIC(rk808_clk_out),
the PMIC output the xin32k seem isn't readied before get the clk.

If the PMIC(rk808) driver is more front loading,I believe it will hasn't 
this prblem.

>> +	ret = clk_set_rate(data->clk, 10000);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "failed to set tsadc clk rate, %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_prepare_enable(data->clk);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "failed to enable converter clock\n");
>> +		goto err_clk;
>> +	}
>> +
>> +	ret = clk_prepare_enable(data->pclk);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "failed to enable pclk\n");
>> +		goto err_pclk;
>> +	}
>> +
>> +	cpumask_set_cpu(0, &clip_cpus);
>> +	data->cdev = of_cpufreq_cooling_register(np, &clip_cpus);
>> +	if (IS_ERR(data->cdev)) {
>> +		dev_err(&pdev->dev, "failed to register cpufreq cooling device\n");
>> +		goto disable_clk;
>> +	}
>> +
>> +	data->irq = platform_get_irq(pdev, 0);
>> +	if (data->irq < 0) {
>> +		dev_err(&pdev->dev, "no irq resource?\n");
>> +		goto disable_clk;
>> +	}
>> +
>> +	ret = devm_request_threaded_irq(&pdev->dev, data->irq, NULL,
>> +					&rockchip_thermal_alarm_irq_thread,
>> +					IRQF_ONESHOT, "rockchip_thermal",
>> +					data);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev,
>> +			"failed to request tsadc irq: %d\n", ret);
>> +		goto disable_clk;
>> +	}
>> +
>> +	ret = rockchip_configure_from_dt(&pdev->dev, np, data);
>> +	if (ret)
>> +		dev_err(&pdev->dev, "Parsing device tree data error.\n");
>> +
>> +	/* The system three Teperture Sensors be registered */
>> +	for (chn = RESERVE; chn < SENSOR_ID_END; chn++) {
>> +		data->chn = chn;
>> +
>> +		data->tz[chn] = thermal_zone_of_sensor_register(
>> +				&pdev->dev, data->chn,
>> +				data, rockchip_thermal_get_temp,
>> +				NULL,
>> +				rockchip_thermal_set_trips);
>> +		if (IS_ERR(data->tz[chn])) {
>> +			err = PTR_ERR(data->tz[chn]);
>> +			dev_err(&pdev->dev, "failed to register sensor: %d\n",
>> +				err);
>> +			chn--;
>> +			goto unregister_tzs;
>> +		}
> Do we have to have all zones defined? Why?
>

Yes, I think so.

At least, CPU and GPU zone is need.

As the TRM introduce,this system has three temperature sensors.
>> +
>> +		platform_set_drvdata(pdev, data);
>> +
>> +		rockchip_thermal_initialize(data);
>> +		rockchip_thermal_control(data, true);
>> +	}
>> +	return 0;
>> +
>> +unregister_tzs:
>> +	for (; chn >= RESERVE; chn--)
>> +		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
>> +	cpufreq_cooling_unregister(data->cdev);
>> +
>> +disable_clk:
>> +err_pclk:
>> +	clk_disable_unprepare(data->pclk);
>> +err_clk:
>> +	clk_disable_unprepare(data->clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static int rockchip_thermal_remove(struct platform_device *pdev)
>> +{
>> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
>> +	int chn;
>> +
>> +	rockchip_thermal_control(data, false);
>> +
>> +	for (; chn >= RESERVE; chn--)
> You are not initializing chn, it will blow up.

Fixed.
>> +		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
>> +	cpufreq_cooling_unregister(data->cdev);
>> +
>> +	clk_disable_unprepare(data->clk);
>> +	clk_disable_unprepare(data->pclk);
>> +
>> +	return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int rockchip_thermal_suspend(struct device *dev)
>> +{
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
>> +
>> +	rockchip_thermal_control(data, false);
>> +
>> +	clk_disable_unprepare(data->clk);
>> +	clk_disable_unprepare(data->pclk);
> Why do we unpreparing in suspend instead of just disabling?

Fixed.
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_thermal_resume(struct device *dev)
>> +{
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
>> +	int ret;
>> +
>> +	ret = clk_prepare_enable(data->pclk);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = clk_prepare_enable(data->clk);
>> +	if (ret)
>> +		return ret;
>> +
>> +	rockchip_thermal_initialize(data);
>> +	rockchip_thermal_control(data, true);
>> +
>> +	return 0;
>> +}
>> +#endif
>> +
>> +static SIMPLE_DEV_PM_OPS(rockchip_thermal_pm_ops,
>> +			 rockchip_thermal_suspend, rockchip_thermal_resume);
>> +
>> +static struct platform_driver rockchip_thermal_driver = {
>> +	.driver = {
>> +		   .name = "rockchip-thermal",
>> +		   .owner = THIS_MODULE,
>> +		   .pm = &rockchip_thermal_pm_ops,
>> +		   .of_match_table = of_rockchip_thermal_match,
>> +		   },
>> +	.probe = rockchip_thermal_probe,
>> +	.remove = rockchip_thermal_remove,
>> +};
>> +
>> +module_platform_driver(rockchip_thermal_driver);
>> +
>> +MODULE_DESCRIPTION("ROCKCHIP THERMAL Driver");
>> +MODULE_AUTHOR("Rockchip, Inc.");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("platform:rockchip-thermal");
>> -- 
>> 1.9.1
>>
>>
> Thanks.
>

-- 
Best regards,
Caesar



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v8 1/5] thermal: rockchip: add driver for thermal
@ 2014-10-11  6:01       ` Caesar Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Caesar Wang @ 2014-10-11  6:01 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Dmitry,

? 2014?10?11? 01:46, Dmitry Torokhov ??:
> Hi Caesar,
>
> On Fri, Oct 10, 2014 at 06:19:37PM +0800, Caesar Wang wrote:
>> Thermal is TS-ADC Controller module supports
>> user-defined mode and automatic mode.
>>
>> User-defined mode refers,TSADC all the control signals entirely by
>> software writing to register for direct control.
>>
>> Automaic mode refers to the module automatically poll TSADC output,
>> and the results were checked.If you find that the temperature High
>> in a period of time,an interrupt is generated to the processor
>> down-measures taken;If the temperature over a period of time High,
>> the resulting TSHUT gave CRU module,let it reset the entire chip,
>> or via GPIO give PMIC.
>>
>> Signed-off-by: zhaoyifeng <zyf@rock-chips.com>
>> Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
>> ---
>>   drivers/thermal/Kconfig            |   9 +
>>   drivers/thermal/Makefile           |   1 +
>>   drivers/thermal/rockchip_thermal.c | 628 +++++++++++++++++++++++++++++++++++++
>>   3 files changed, 638 insertions(+)
>>   create mode 100644 drivers/thermal/rockchip_thermal.c
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index f9a1386..24845ae 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -133,6 +133,15 @@ config SPEAR_THERMAL
>>   	  Enable this to plug the SPEAr thermal sensor driver into the Linux
>>   	  thermal framework.
>>   
>> +config ROCKCHIP_THERMAL
>> +	tristate "Rockchip thermal driver"
>> +	depends on ARCH_ROCKCHIP
>> +	help
>> +	  Rockchip thermal driver provides support for Temperature sensor
>> +	  ADC (TS-ADC) found on Rockchip SoCs. It supports one critical
>> +	  trip point. Cpufreq is used as the cooling device and will throttle
>> +	  CPUs when the Temperature crosses the passive trip point.
>> +
>>   config RCAR_THERMAL
>>   	tristate "Renesas R-Car thermal driver"
>>   	depends on ARCH_SHMOBILE || COMPILE_TEST
>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>> index de0636a..930554f 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -19,6 +19,7 @@ thermal_sys-$(CONFIG_CPU_THERMAL)	+= cpu_cooling.o
>>   
>>   # platform thermal drivers
>>   obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
>> +obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
>>   obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
>>   obj-$(CONFIG_KIRKWOOD_THERMAL)  += kirkwood_thermal.o
>>   obj-y				+= samsung/
>> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
>> new file mode 100644
>> index 0000000..ceee2c1
>> --- /dev/null
>> +++ b/drivers/thermal/rockchip_thermal.c
>> @@ -0,0 +1,628 @@
>> +/*
>> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/cpu_cooling.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/thermal.h>
>> +
>> +/**
>> +* If the temperature over a period of time High,
>> +* the resulting TSHUT gave CRU module,let it reset the entire chip,
>> +* or via GPIO give PMIC.
>> +*/
>> +enum reset_mde {
> Can we spare one more letter? ;)
>

OK, maybe fix it as the follows:

enum reset_mde {
     TSADC_HT_RESET_CRU = 0,
     TSADC_HT_PULL_GPIO,
};
>> +	CRU = 0,
>> +	GPIO,
>> +};
>> +
>> +/**
>> +* The system has three Teperture Sensors.
>> +* channel0 is reserve,channel1 is for CPU,and
>> +* channel channel 2 is for GPU.
>> +*/
>> +enum sensor_id {
>> +	RESERVE = 0,
>> +	CPU,
>> +	GPU,
>> +	SENSOR_ID_END,
>> +};
>> +
>> +struct rockchip_thermal_data {
>> +	const struct rockchip_tsadc_platform_data *pdata;
>> +	struct thermal_zone_device *tz[SENSOR_ID_END];
>> +	struct thermal_cooling_device *cdev;
>> +	void __iomem *regs;
>> +
>> +	unsigned long temp_passive;
>> +	unsigned long hw_shut_temp;
>> +	unsigned long alarm_temp;
>> +	bool irq_enabled;
>> +	int irq;
>> +	int reset_mode;
>> +	int chn;
>> +
>> +	struct clk *clk;
>> +	struct clk *pclk;
>> +};
>> +
>> +struct rockchip_tsadc_platform_data {
>> +	unsigned long temp_passive;
>> +	unsigned long hw_shut_temp;
>> +	int reset_mode;
>> +
>> +	int (*irq_handle)(void __iomem *reg);
> Why does it return int and not void? What is the return value of this?

Fixed. instead of using void.
>> +	int (*initialize)(int reset_mode, int chn, void __iomem *reg,
> Why does it return int and not void? What is the return value of this?> +			  unsigned long hw_shut_temp);

Ditto.

>> +	int (*control)(void __iomem *reg, bool on);
>> +	int (*code_to_temp)(u32 code);
>> +	u32 (*temp_to_code)(int temp);
>> +	void (*set_alarm_temp)(int chn, void __iomem *reg,
>> +			       unsigned long alarm_temp);
>> +};
>> +
>> +/* TSADC V2 Sensor info define: */
>> +#define TSADCV2_AUTO_CON			0x04
>> +#define TSADCV2_INT_EN				0x08
>> +#define TSADCV2_INT_PD				0x0c
>> +#define TSADCV2_DATA(chn)			(0x20+chn*0x04)
> Unsafe macros. You want to enclose argument in parenthesis.

Yeah.
Hmmm, there is no thought of a good way to deal with it at the moment.

Maybe do that as the follows,but should be a better way to deal.

e.g:
#define TSADCV2_DATA0  0x20
#define TSADCV2_DATA1  0x24
#define TSADCV2_DATA2  0x28
#define TSADCV2_DATA3  0x2c

#define TSADCV2_COMP0_INT  0x30
#define TSADCV2_COMP1_INT  0x34
#define TSADCV2_COMP2_INT  0x38
#define TSADCV2_COMP3_INT  0x3c

#define TSADCV2_COMP0_SHUT  0x40
#define TSADCV2_COMP1_SHUT  0x44
#define TSADCV2_COMP2_SHUT  0x48
#define TSADCV2_COMP3_SHUT  0x4c
>> +#define TSADCV2_COMP_INT(chn)		        (0x30+chn*0x04)
>> +#define TSADCV2_COMP_SHUT(chn)		        (0x40+chn*0x04)
>> +#define TSADCV2_HIGHT_INT_DEBOUNCE		0x60
>> +#define TSADCV2_HIGHT_TSHUT_DEBOUNCE		0x64
>> +#define TSADCV2_AUTO_PERIOD			0x68
>> +#define TSADCV2_AUTO_PERIOD_HT			0x6c
>> +
>> +#define TSADCV2_AUTO_EN				BIT(0)
>> +#define TSADCV2_AUTO_DISABLE			~BIT(0)
>> +#define TSADCV2_AUTO_SRC_EN(chn)	        (0xf << (4 + chn))
>> +#define TSADCV2_AUTO_TSHUT_POLARITY_HIGH	BIT(8)
>> +
>> +#define TSADCV2_INT_SRC_EN(chn)			BIT(chn)
>> +#define TSADCV2_SHUT_2GPIO_SRC_EN(chn)		(0xf << (4 + chn))
>> +#define TSADCV2_SHUT_2CRU_SRC_EN(chn)		(0xf << (8 + chn))
>> +
>> +#define TSADCV2_INT_PD_CLEAR			~BIT(8)
>> +
>> +#define TSADCV2_DATA_MASK			0xfff
>> +#define TSADCV2_HIGHT_INT_DEBOUNCE_TIME		0x0a
>> +#define TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME	0x0a
>> +#define TSADCV2_AUTO_PERIOD_TIME		0x03e8
>> +#define TSADCV2_AUTO_PERIOD_HT_TIME		0x64
>> +
>> +struct tsadc_table {
>> +	unsigned long code;
>> +	int temp;
>> +};
>> +
>> +static const struct tsadc_table v2_code_table[] = {
>> +	{TSADCV2_DATA_MASK, -40000},
>> +	{3800, -40000},
>> +	{3792, -35000},
>> +	{3783, -30000},
>> +	{3774, -25000},
>> +	{3765, -20000},
>> +	{3756, -15000},
>> +	{3747, -10000},
>> +	{3737, -5000},
>> +	{3728, 0},
>> +	{3718, 5000},
>> +	{3708, 10000},
>> +	{3698, 15000},
>> +	{3688, 20000},
>> +	{3678, 25000},
>> +	{3667, 30000},
>> +	{3656, 35000},
>> +	{3645, 40000},
>> +	{3634, 45000},
>> +	{3623, 50000},
>> +	{3611, 55000},
>> +	{3600, 60000},
>> +	{3588, 65000},
>> +	{3575, 70000},
>> +	{3563, 75000},
>> +	{3550, 80000},
>> +	{3537, 85000},
>> +	{3524, 90000},
>> +	{3510, 95000},
>> +	{3496, 100000},
>> +	{3482, 105000},
>> +	{3467, 110000},
>> +	{3452, 115000},
>> +	{3437, 120000},
>> +	{3421, 125000},
>> +	{0, 125000},
>> +};
>> +
>> +static int rk_tsadcv2_irq_handle(void __iomem *regs)
>> +{
>> +	u32 val;
>> +
>> +	val = readl_relaxed(regs + TSADCV2_INT_PD);
>> +	writel_relaxed(val & TSADCV2_INT_PD_CLEAR, regs + TSADCV2_INT_PD);
>> +
>> +	return 0;
>> +}
>> +
>> +static u32 rk_tsadcv2_temp_to_code(int temp)
>> +{
>> +	int high, low, mid, ret = 0;
>> +
>> +	low = 0;
>> +	high = ARRAY_SIZE(v2_code_table) - 1;
>> +	mid = (high + low) / 2;
>> +
>> +	if (temp < v2_code_table[low].temp || temp > v2_code_table[high].temp) {
>> +		ret = -ERANGE;
> The function returns u32, you can't return -ERANGE here.

Yeah,fixed.
>> +		goto exit;
>> +	}
>> +
>> +	while (low <= high) {
>> +		if (temp == v2_code_table[mid].temp)
>> +			return v2_code_table[mid].code;
>> +		else if (temp < v2_code_table[mid].temp)
>> +			high = mid - 1;
>> +		else
>> +			low = mid + 1;
>> +		mid = (low + high) / 2;
>> +	}
> Hmm, you optimized temp_to_code but not code_to_temp, why?

OK,fixed.
>> +
>> +	return 0;
>> +
>> +exit:
>> +	return ret;
>> +}
>> +
>> +static int rk_tsadcv2_code_to_temp(u32 code)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(v2_code_table) - 1; i++) {
>> +		if (code >= v2_code_table[i].code)
>> +			return v2_code_table[i].temp;
>> +	}
>> +
>> +	/* No code available,return max temperture */
>> +	return 125000;
>> +}
>> +
>> +static int rk_tsadcv2_initialize(int reset_mode, int chn, void __iomem *regs,
>> +				 unsigned long hw_shut_temp)
>> +{
>> +	u32 shutdown_value;
>> +
>> +	shutdown_value = rk_tsadcv2_temp_to_code(hw_shut_temp);
>> +
>> +	/* Enable measurements at ~ 10 Hz */
>> +	writel_relaxed(0 | TSADCV2_AUTO_TSHUT_POLARITY_HIGH, regs +
>> +		       TSADCV2_AUTO_CON);
>> +	writel_relaxed(TSADCV2_AUTO_PERIOD_TIME, regs + TSADCV2_AUTO_PERIOD);
>> +	writel_relaxed(TSADCV2_AUTO_PERIOD_HT_TIME, regs +
>> +		       TSADCV2_AUTO_PERIOD_HT);
>> +	writel_relaxed(shutdown_value, regs + TSADCV2_COMP_SHUT(chn));
>> +	writel_relaxed(TSADCV2_HIGHT_INT_DEBOUNCE_TIME, regs +
>> +		       TSADCV2_HIGHT_INT_DEBOUNCE);
>> +	writel_relaxed(TSADCV2_HIGHT_TSHUT_DEBOUNCE_TIME, regs +
>> +		       TSADCV2_HIGHT_TSHUT_DEBOUNCE);
>> +
>> +	if (reset_mode == GPIO)
>> +		writel_relaxed(TSADCV2_SHUT_2GPIO_SRC_EN(chn) |
>> +			       TSADCV2_INT_SRC_EN(chn), regs +
>> +			       TSADCV2_INT_EN);
>> +	else
>> +		writel_relaxed(TSADCV2_SHUT_2CRU_SRC_EN(chn) |
>> +			       TSADCV2_INT_SRC_EN(chn) , regs +
>> +			       TSADCV2_INT_EN);
>> +
>> +	writel_relaxed(TSADCV2_AUTO_SRC_EN(chn) | TSADCV2_AUTO_EN, regs +
>> +		       TSADCV2_AUTO_CON);
>> +
>> +	return 0;
>> +}
>> +
>> +static int rk_tsadcv2_control(void __iomem *regs, bool on)
>> +{
>> +	u32 val;
>> +
>> +	if (on) {
>> +		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
>> +		writel_relaxed(val | TSADCV2_AUTO_EN, regs + TSADCV2_AUTO_CON);
>> +	} else {
>> +		val = readl_relaxed(regs + TSADCV2_AUTO_CON);
>> +		writel_relaxed(val & TSADCV2_AUTO_DISABLE,
>> +			       regs + TSADCV2_AUTO_CON);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void rk_tsadcv2_alarm_temp(int chn, void __iomem *regs,
>> +				  unsigned long alarm_temp)
>> +{
>> +	u32 alarm_value;
>> +
>> +	alarm_value = rk_tsadcv2_temp_to_code(alarm_temp);
>> +
>> +	writel_relaxed(alarm_value & TSADCV2_DATA_MASK, regs +
>> +		       TSADCV2_COMP_INT(chn));
>> +}
>> +
>> +static const struct rockchip_tsadc_platform_data rk3288_tsadc_data = {
>> +	.reset_mode = GPIO, /* default TSHUT via GPIO give PMIC */
>> +	.temp_passive = 80000,
>> +	.hw_shut_temp = 120000,
>> +	.irq_handle = rk_tsadcv2_irq_handle,
>> +	.initialize = rk_tsadcv2_initialize,
>> +	.control = rk_tsadcv2_control,
>> +	.code_to_temp = rk_tsadcv2_code_to_temp,
>> +	.temp_to_code = rk_tsadcv2_temp_to_code,
>> +	.set_alarm_temp = rk_tsadcv2_alarm_temp,
>> +};
>> +
>> +static const struct of_device_id of_rockchip_thermal_match[] = {
>> +	{
>> +		.compatible = "rockchip,rk3288-tsadc",
>> +		.data = (void *)&rk3288_tsadc_data,
>> +	},
>> +	{ /* end */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
>> +
>> +static void rockchip_set_alarm_temp(struct rockchip_thermal_data *data,
>> +				    int alarm_temp)
>> +{
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +
>> +	data->alarm_temp = alarm_temp;
>> +	if (tsadc->set_alarm_temp)
>> +		tsadc->set_alarm_temp(data->chn, data->regs, alarm_temp);
>> +}
>> +
>> +static int rockchip_thermal_initialize(struct rockchip_thermal_data *data)
>> +{
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +
>> +	if (data->chn == RESERVE)
>> +		return 0;
>> +
>> +	if (tsadc->initialize)
>> +		tsadc->initialize(tsadc->reset_mode, data->chn, data->regs,
>> +				  data->hw_shut_temp);
>> +
>> +	rockchip_set_alarm_temp(data, data->temp_passive);
>> +
>> +	return 0;
>> +}
>> +
>> +static void rockchip_thermal_control(struct rockchip_thermal_data *data,
>> +				     bool on)
>> +{
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +
>> +	if (tsadc->control)
>> +		tsadc->control(data->regs, on);
>> +
>> +	if (on) {
>> +		data->irq_enabled = true;
>> +		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
>> +		THERMAL_DEVICE_ENABLED);
>> +	} else {
>> +		data->irq_enabled = false;
>> +		data->tz[data->chn]->ops->set_mode(data->tz[data->chn],
>> +		THERMAL_DEVICE_DISABLED);
>> +	}
>> +}
>> +
>> +static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
>> +{
>> +	struct rockchip_thermal_data *data = dev;
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +	int chn;
>> +
>> +	if (tsadc->irq_handle)
>> +		tsadc->irq_handle(data->regs);
>> +
>> +	for (chn = CPU; chn < SENSOR_ID_END; chn++)
>> +		thermal_zone_device_update(data->tz[chn]);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int rockchip_thermal_set_trips(void *zone, long low, long high)
>> +{
>> +	struct rockchip_thermal_data *data = zone;
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +	u32 val;
>> +
>> +	low = clamp_val(low, LONG_MIN, LONG_MAX);
>> +	high = clamp_val(high, LONG_MIN, LONG_MAX);
>> +
>> +	/* channel0 is RESERVE, not need to set trips*/
>> +	if (data->chn == RESERVE)
>> +		return 0;
>> +
>> +	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));
> I do not think you need to re-read the temperature: you are already
> goivent lower and upper trip points for current temperature, you just
> need to set alarm to the upper one.
>

Fixed.
>> +	if (val == 0)
>> +		return -EPROBE_DEFER;
> What -EPROBE_DEFER means in this context?
>

In general, the A/D value of the channel last conversion need some time.
There isn't a need in here.
>> +
>> +	/* Update alarm value to next higher trip point */
>> +	if (data->alarm_temp == data->temp_passive && val <=
>> +		tsadc->temp_to_code(data->temp_passive))
>> +		high = data->hw_shut_temp;
>> +
>> +	if (data->alarm_temp >= data->temp_passive && val >
>> +		tsadc->temp_to_code(data->temp_passive)) {
>> +		high = data->temp_passive;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_thermal_get_temp(void *zone, long *out_temp)
>> +{
>> +	struct rockchip_thermal_data *data = zone;
>> +	const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
>> +	u32 val;
>> +
>> +	if (data->chn == RESERVE)
>> +		return 0;
>> +
>> +	val = readl_relaxed(data->regs + TSADCV2_DATA(data->chn));
>> +
>> +	if (val < tsadc->temp_to_code(data->hw_shut_temp))
>> +		*out_temp = 120000;
> Why is this needed? If this is really needed please comment the code.

Fixed.
It's not need.

>> +	else
>> +		*out_temp = tsadc->code_to_temp(val);
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_configure_from_dt(struct device *dev,
>> +				      struct device_node *np,
>> +				      struct rockchip_thermal_data *data)
>> +{
>> +	int shut_temp, reset_mode;
>> +
>> +	if (of_property_read_u32(np, "hw-shut-temp", &shut_temp)) {
>> +		dev_warn(dev, "Missing default shutdown temp property\n");
>> +		data->hw_shut_temp = data->pdata->hw_shut_temp;
> I am still not sure why we want it in DT. I envision the set up as
> follows:
>
> passive trip point - defined in DT
> critical trip point - defined in DT, causes shutdown performed by
> thermal core for us
> hardware critical - hard coded (defined by the hardware, no point in
> changing), shuts down or resets the box automatically by the hardware,
> no kernel involved.

Although TSHUT(hardware shutdown temperature value) is not a generic 
trip point,
TSHUT value is relative fixed and need be init as the tsadc_comp.

hmm..,if  it doeen't define in the DT,we had to get a TSHUT  from data,

as the about:
dev_warn(dev, "Missing default shutdown temp property\n");
		data->hw_shut_temp = data->pdata->hw_shut_temp;



>> +	} else {
>> +		data->hw_shut_temp = shut_temp;
>> +	}
>> +
>> +	if (of_property_read_u32(np, "tsadc-ht-reset-mode", &reset_mode)) {
>> +		dev_warn(dev, "Missing default reset mode property\n");
>> +		data->reset_mode = data->pdata->reset_mode;
>> +	} else {
>> +		data->reset_mode = reset_mode;
>> +	}
>> +
>> +	data->temp_passive = data->pdata->temp_passive;
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_thermal_probe(struct platform_device *pdev)
>> +{
>> +	struct rockchip_thermal_data *data;
>> +	const struct rockchip_tsadc_platform_data *tsadc;
>> +	const struct of_device_id *match;
>> +
>> +	struct cpumask clip_cpus;
>> +	struct resource *res;
>> +	struct device_node *np = pdev->dev.of_node;
>> +
>> +	int ret, err, chn;
>> +
>> +	data = devm_kzalloc(&pdev->dev, sizeof(struct rockchip_thermal_data),
>> +			    GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	data->regs = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(data->regs)) {
>> +		dev_err(&pdev->dev, "Could not get tsadc source, %p\n",
>> +			data->regs);
>> +		return PTR_ERR(data->regs);
>> +	}
>> +
>> +	match = of_match_node(of_rockchip_thermal_match, np);
>> +	if (!match)
>> +		return -ENXIO;
>> +	data->pdata = (const struct rockchip_tsadc_platform_data *)match->data;
>> +	if (!data->pdata)
>> +		return -EINVAL;
>> +	tsadc = data->pdata;
>> +
>> +	data->clk = devm_clk_get(&pdev->dev, "tsadc");
>> +	if (IS_ERR(data->clk)) {
>> +		dev_err(&pdev->dev, "failed to get tsadc clock\n");
>> +		return PTR_ERR(data->clk);
>> +	}
>> +
>> +	data->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
>> +	if (IS_ERR(data->pclk)) {
>> +		dev_err(&pdev->dev, "failed to get tsadc pclk\n");
>> +		return PTR_ERR(data->pclk);
>> +	}
>> +
>> +	/**
>> +	* Use a default of 10KHz for the converter clock.
>> +	* This may become user-configurable in the future.
>> +	* Need to judge the data->clk is Divided by xin32k.
>> +	* Need to retry it if the pmic hasn't output the xin32k.
>> +	*/
>> +	if (clk_get_rate(data->clk) == 0)
>> +		return -EPROBE_DEFER;
> What would cause the clock to be present but report 0 rate? Are we sure
> that it will start reporting different data after binding some more
> drivers?
>

The clk is divided by xin32k from PMIC(rk808_clk_out),
the PMIC output the xin32k seem isn't readied before get the clk.

If the PMIC(rk808) driver is more front loading,I believe it will hasn't 
this prblem.

>> +	ret = clk_set_rate(data->clk, 10000);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "failed to set tsadc clk rate, %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_prepare_enable(data->clk);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "failed to enable converter clock\n");
>> +		goto err_clk;
>> +	}
>> +
>> +	ret = clk_prepare_enable(data->pclk);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "failed to enable pclk\n");
>> +		goto err_pclk;
>> +	}
>> +
>> +	cpumask_set_cpu(0, &clip_cpus);
>> +	data->cdev = of_cpufreq_cooling_register(np, &clip_cpus);
>> +	if (IS_ERR(data->cdev)) {
>> +		dev_err(&pdev->dev, "failed to register cpufreq cooling device\n");
>> +		goto disable_clk;
>> +	}
>> +
>> +	data->irq = platform_get_irq(pdev, 0);
>> +	if (data->irq < 0) {
>> +		dev_err(&pdev->dev, "no irq resource?\n");
>> +		goto disable_clk;
>> +	}
>> +
>> +	ret = devm_request_threaded_irq(&pdev->dev, data->irq, NULL,
>> +					&rockchip_thermal_alarm_irq_thread,
>> +					IRQF_ONESHOT, "rockchip_thermal",
>> +					data);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev,
>> +			"failed to request tsadc irq: %d\n", ret);
>> +		goto disable_clk;
>> +	}
>> +
>> +	ret = rockchip_configure_from_dt(&pdev->dev, np, data);
>> +	if (ret)
>> +		dev_err(&pdev->dev, "Parsing device tree data error.\n");
>> +
>> +	/* The system three Teperture Sensors be registered */
>> +	for (chn = RESERVE; chn < SENSOR_ID_END; chn++) {
>> +		data->chn = chn;
>> +
>> +		data->tz[chn] = thermal_zone_of_sensor_register(
>> +				&pdev->dev, data->chn,
>> +				data, rockchip_thermal_get_temp,
>> +				NULL,
>> +				rockchip_thermal_set_trips);
>> +		if (IS_ERR(data->tz[chn])) {
>> +			err = PTR_ERR(data->tz[chn]);
>> +			dev_err(&pdev->dev, "failed to register sensor: %d\n",
>> +				err);
>> +			chn--;
>> +			goto unregister_tzs;
>> +		}
> Do we have to have all zones defined? Why?
>

Yes, I think so.

At least, CPU and GPU zone is need.

As the TRM introduce,this system has three temperature sensors.
>> +
>> +		platform_set_drvdata(pdev, data);
>> +
>> +		rockchip_thermal_initialize(data);
>> +		rockchip_thermal_control(data, true);
>> +	}
>> +	return 0;
>> +
>> +unregister_tzs:
>> +	for (; chn >= RESERVE; chn--)
>> +		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
>> +	cpufreq_cooling_unregister(data->cdev);
>> +
>> +disable_clk:
>> +err_pclk:
>> +	clk_disable_unprepare(data->pclk);
>> +err_clk:
>> +	clk_disable_unprepare(data->clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static int rockchip_thermal_remove(struct platform_device *pdev)
>> +{
>> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
>> +	int chn;
>> +
>> +	rockchip_thermal_control(data, false);
>> +
>> +	for (; chn >= RESERVE; chn--)
> You are not initializing chn, it will blow up.

Fixed.
>> +		thermal_zone_of_sensor_unregister(&pdev->dev, data->tz[chn]);
>> +	cpufreq_cooling_unregister(data->cdev);
>> +
>> +	clk_disable_unprepare(data->clk);
>> +	clk_disable_unprepare(data->pclk);
>> +
>> +	return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int rockchip_thermal_suspend(struct device *dev)
>> +{
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
>> +
>> +	rockchip_thermal_control(data, false);
>> +
>> +	clk_disable_unprepare(data->clk);
>> +	clk_disable_unprepare(data->pclk);
> Why do we unpreparing in suspend instead of just disabling?

Fixed.
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_thermal_resume(struct device *dev)
>> +{
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	struct rockchip_thermal_data *data = platform_get_drvdata(pdev);
>> +	int ret;
>> +
>> +	ret = clk_prepare_enable(data->pclk);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = clk_prepare_enable(data->clk);
>> +	if (ret)
>> +		return ret;
>> +
>> +	rockchip_thermal_initialize(data);
>> +	rockchip_thermal_control(data, true);
>> +
>> +	return 0;
>> +}
>> +#endif
>> +
>> +static SIMPLE_DEV_PM_OPS(rockchip_thermal_pm_ops,
>> +			 rockchip_thermal_suspend, rockchip_thermal_resume);
>> +
>> +static struct platform_driver rockchip_thermal_driver = {
>> +	.driver = {
>> +		   .name = "rockchip-thermal",
>> +		   .owner = THIS_MODULE,
>> +		   .pm = &rockchip_thermal_pm_ops,
>> +		   .of_match_table = of_rockchip_thermal_match,
>> +		   },
>> +	.probe = rockchip_thermal_probe,
>> +	.remove = rockchip_thermal_remove,
>> +};
>> +
>> +module_platform_driver(rockchip_thermal_driver);
>> +
>> +MODULE_DESCRIPTION("ROCKCHIP THERMAL Driver");
>> +MODULE_AUTHOR("Rockchip, Inc.");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("platform:rockchip-thermal");
>> -- 
>> 1.9.1
>>
>>
> Thanks.
>

-- 
Best regards,
Caesar

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v8 1/5] thermal: rockchip: add driver for thermal
  2014-10-11  6:01       ` Caesar Wang
@ 2014-10-11  6:20         ` Dmitry Torokhov
  -1 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2014-10-11  6:20 UTC (permalink / raw)
  To: Caesar Wang
  Cc: heiko, rui.zhang, edubezval, arnd, zyf, dianders, linux-rockchip,
	linux-kernel, linux-pm, linux-arm-kernel, devicetree, linux-doc,
	cf, dbasehore, huangtao, cjf, zhengsq

Hi Caesar,

On Sat, Oct 11, 2014 at 02:01:33PM +0800, Caesar Wang wrote:
> Dear Dmitry,
> 
> 在 2014年10月11日 01:46, Dmitry Torokhov 写道:
> >Hi Caesar,
> >
> >On Fri, Oct 10, 2014 at 06:19:37PM +0800, Caesar Wang wrote:
> >>Thermal is TS-ADC Controller module supports
> >>user-defined mode and automatic mode.
> >>
> >>User-defined mode refers,TSADC all the control signals entirely by
> >>software writing to register for direct control.
> >>
> >>Automaic mode refers to the module automatically poll TSADC output,
> >>and the results were checked.If you find that the temperature High
> >>in a period of time,an interrupt is generated to the processor
> >>down-measures taken;If the temperature over a period of time High,
> >>the resulting TSHUT gave CRU module,let it reset the entire chip,
> >>or via GPIO give PMIC.
> >>
> >>Signed-off-by: zhaoyifeng <zyf@rock-chips.com>
> >>Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
> >>---
> >>  drivers/thermal/Kconfig            |   9 +
> >>  drivers/thermal/Makefile           |   1 +
> >>  drivers/thermal/rockchip_thermal.c | 628 +++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 638 insertions(+)
> >>  create mode 100644 drivers/thermal/rockchip_thermal.c
> >>
> >>diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> >>index f9a1386..24845ae 100644
> >>--- a/drivers/thermal/Kconfig
> >>+++ b/drivers/thermal/Kconfig
> >>@@ -133,6 +133,15 @@ config SPEAR_THERMAL
> >>  	  Enable this to plug the SPEAr thermal sensor driver into the Linux
> >>  	  thermal framework.
> >>+config ROCKCHIP_THERMAL
> >>+	tristate "Rockchip thermal driver"
> >>+	depends on ARCH_ROCKCHIP
> >>+	help
> >>+	  Rockchip thermal driver provides support for Temperature sensor
> >>+	  ADC (TS-ADC) found on Rockchip SoCs. It supports one critical
> >>+	  trip point. Cpufreq is used as the cooling device and will throttle
> >>+	  CPUs when the Temperature crosses the passive trip point.
> >>+
> >>  config RCAR_THERMAL
> >>  	tristate "Renesas R-Car thermal driver"
> >>  	depends on ARCH_SHMOBILE || COMPILE_TEST
> >>diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> >>index de0636a..930554f 100644
> >>--- a/drivers/thermal/Makefile
> >>+++ b/drivers/thermal/Makefile
> >>@@ -19,6 +19,7 @@ thermal_sys-$(CONFIG_CPU_THERMAL)	+= cpu_cooling.o
> >>  # platform thermal drivers
> >>  obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
> >>+obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
> >>  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
> >>  obj-$(CONFIG_KIRKWOOD_THERMAL)  += kirkwood_thermal.o
> >>  obj-y				+= samsung/
> >>diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> >>new file mode 100644
> >>index 0000000..ceee2c1
> >>--- /dev/null
> >>+++ b/drivers/thermal/rockchip_thermal.c
> >>@@ -0,0 +1,628 @@
> >>+/*
> >>+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> >>+ *
> >>+ * This program is free software; you can redistribute it and/or modify it
> >>+ * under the terms and conditions of the GNU General Public License,
> >>+ * version 2, as published by the Free Software Foundation.
> >>+ *
> >>+ * This program is distributed in the hope 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.
> >>+ */
> >>+
> >>+#include <linux/clk.h>
> >>+#include <linux/cpu_cooling.h>
> >>+#include <linux/interrupt.h>
> >>+#include <linux/io.h>
> >>+#include <linux/module.h>
> >>+#include <linux/of.h>
> >>+#include <linux/of_address.h>
> >>+#include <linux/of_irq.h>
> >>+#include <linux/platform_device.h>
> >>+#include <linux/thermal.h>
> >>+
> >>+/**
> >>+* If the temperature over a period of time High,
> >>+* the resulting TSHUT gave CRU module,let it reset the entire chip,
> >>+* or via GPIO give PMIC.
> >>+*/
> >>+enum reset_mde {
> >Can we spare one more letter? ;)
> >
> 
> OK, maybe fix it as the follows:
> 
> enum reset_mde {
>     TSADC_HT_RESET_CRU = 0,
>     TSADC_HT_PULL_GPIO,
> };

I meant to spell "mode" out, like this:

enum reset_mode {
	...
};

> >>+	CRU = 0,
> >>+	GPIO,
> >>+};
> >>+
> >>+/**
> >>+* The system has three Teperture Sensors.
> >>+* channel0 is reserve,channel1 is for CPU,and
> >>+* channel channel 2 is for GPU.
> >>+*/
> >>+enum sensor_id {
> >>+	RESERVE = 0,
> >>+	CPU,
> >>+	GPU,
> >>+	SENSOR_ID_END,
> >>+};
> >>+
> >>+struct rockchip_thermal_data {
> >>+	const struct rockchip_tsadc_platform_data *pdata;
> >>+	struct thermal_zone_device *tz[SENSOR_ID_END];
> >>+	struct thermal_cooling_device *cdev;
> >>+	void __iomem *regs;
> >>+
> >>+	unsigned long temp_passive;
> >>+	unsigned long hw_shut_temp;
> >>+	unsigned long alarm_temp;
> >>+	bool irq_enabled;
> >>+	int irq;
> >>+	int reset_mode;
> >>+	int chn;
> >>+
> >>+	struct clk *clk;
> >>+	struct clk *pclk;
> >>+};
> >>+
> >>+struct rockchip_tsadc_platform_data {
> >>+	unsigned long temp_passive;
> >>+	unsigned long hw_shut_temp;
> >>+	int reset_mode;
> >>+
> >>+	int (*irq_handle)(void __iomem *reg);
> >Why does it return int and not void? What is the return value of this?
> 
> Fixed. instead of using void.
> >>+	int (*initialize)(int reset_mode, int chn, void __iomem *reg,
> >Why does it return int and not void? What is the return value of this?> +			  unsigned long hw_shut_temp);
> 
> Ditto.
> 
> >>+	int (*control)(void __iomem *reg, bool on);
> >>+	int (*code_to_temp)(u32 code);
> >>+	u32 (*temp_to_code)(int temp);
> >>+	void (*set_alarm_temp)(int chn, void __iomem *reg,
> >>+			       unsigned long alarm_temp);
> >>+};
> >>+
> >>+/* TSADC V2 Sensor info define: */
> >>+#define TSADCV2_AUTO_CON			0x04
> >>+#define TSADCV2_INT_EN				0x08
> >>+#define TSADCV2_INT_PD				0x0c
> >>+#define TSADCV2_DATA(chn)			(0x20+chn*0x04)
> >Unsafe macros. You want to enclose argument in parenthesis.
> 
> Yeah.
> Hmmm, there is no thought of a good way to deal with it at the moment.
> 
> Maybe do that as the follows,but should be a better way to deal.

No, I meant

#define TSADCV2_DATA(chn)	(0x20 + (chn) * 0x04)

This way one can safely pass expressions as argument, like
TSADCV2_DATA(i + 1) will evaluate properly.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v8 1/5] thermal: rockchip: add driver for thermal
@ 2014-10-11  6:20         ` Dmitry Torokhov
  0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2014-10-11  6:20 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Caesar,

On Sat, Oct 11, 2014 at 02:01:33PM +0800, Caesar Wang wrote:
> Dear Dmitry,
> 
> ? 2014?10?11? 01:46, Dmitry Torokhov ??:
> >Hi Caesar,
> >
> >On Fri, Oct 10, 2014 at 06:19:37PM +0800, Caesar Wang wrote:
> >>Thermal is TS-ADC Controller module supports
> >>user-defined mode and automatic mode.
> >>
> >>User-defined mode refers,TSADC all the control signals entirely by
> >>software writing to register for direct control.
> >>
> >>Automaic mode refers to the module automatically poll TSADC output,
> >>and the results were checked.If you find that the temperature High
> >>in a period of time,an interrupt is generated to the processor
> >>down-measures taken;If the temperature over a period of time High,
> >>the resulting TSHUT gave CRU module,let it reset the entire chip,
> >>or via GPIO give PMIC.
> >>
> >>Signed-off-by: zhaoyifeng <zyf@rock-chips.com>
> >>Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com>
> >>---
> >>  drivers/thermal/Kconfig            |   9 +
> >>  drivers/thermal/Makefile           |   1 +
> >>  drivers/thermal/rockchip_thermal.c | 628 +++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 638 insertions(+)
> >>  create mode 100644 drivers/thermal/rockchip_thermal.c
> >>
> >>diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> >>index f9a1386..24845ae 100644
> >>--- a/drivers/thermal/Kconfig
> >>+++ b/drivers/thermal/Kconfig
> >>@@ -133,6 +133,15 @@ config SPEAR_THERMAL
> >>  	  Enable this to plug the SPEAr thermal sensor driver into the Linux
> >>  	  thermal framework.
> >>+config ROCKCHIP_THERMAL
> >>+	tristate "Rockchip thermal driver"
> >>+	depends on ARCH_ROCKCHIP
> >>+	help
> >>+	  Rockchip thermal driver provides support for Temperature sensor
> >>+	  ADC (TS-ADC) found on Rockchip SoCs. It supports one critical
> >>+	  trip point. Cpufreq is used as the cooling device and will throttle
> >>+	  CPUs when the Temperature crosses the passive trip point.
> >>+
> >>  config RCAR_THERMAL
> >>  	tristate "Renesas R-Car thermal driver"
> >>  	depends on ARCH_SHMOBILE || COMPILE_TEST
> >>diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> >>index de0636a..930554f 100644
> >>--- a/drivers/thermal/Makefile
> >>+++ b/drivers/thermal/Makefile
> >>@@ -19,6 +19,7 @@ thermal_sys-$(CONFIG_CPU_THERMAL)	+= cpu_cooling.o
> >>  # platform thermal drivers
> >>  obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
> >>+obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
> >>  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
> >>  obj-$(CONFIG_KIRKWOOD_THERMAL)  += kirkwood_thermal.o
> >>  obj-y				+= samsung/
> >>diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> >>new file mode 100644
> >>index 0000000..ceee2c1
> >>--- /dev/null
> >>+++ b/drivers/thermal/rockchip_thermal.c
> >>@@ -0,0 +1,628 @@
> >>+/*
> >>+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> >>+ *
> >>+ * This program is free software; you can redistribute it and/or modify it
> >>+ * under the terms and conditions of the GNU General Public License,
> >>+ * version 2, as published by the Free Software Foundation.
> >>+ *
> >>+ * This program is distributed in the hope 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.
> >>+ */
> >>+
> >>+#include <linux/clk.h>
> >>+#include <linux/cpu_cooling.h>
> >>+#include <linux/interrupt.h>
> >>+#include <linux/io.h>
> >>+#include <linux/module.h>
> >>+#include <linux/of.h>
> >>+#include <linux/of_address.h>
> >>+#include <linux/of_irq.h>
> >>+#include <linux/platform_device.h>
> >>+#include <linux/thermal.h>
> >>+
> >>+/**
> >>+* If the temperature over a period of time High,
> >>+* the resulting TSHUT gave CRU module,let it reset the entire chip,
> >>+* or via GPIO give PMIC.
> >>+*/
> >>+enum reset_mde {
> >Can we spare one more letter? ;)
> >
> 
> OK, maybe fix it as the follows:
> 
> enum reset_mde {
>     TSADC_HT_RESET_CRU = 0,
>     TSADC_HT_PULL_GPIO,
> };

I meant to spell "mode" out, like this:

enum reset_mode {
	...
};

> >>+	CRU = 0,
> >>+	GPIO,
> >>+};
> >>+
> >>+/**
> >>+* The system has three Teperture Sensors.
> >>+* channel0 is reserve,channel1 is for CPU,and
> >>+* channel channel 2 is for GPU.
> >>+*/
> >>+enum sensor_id {
> >>+	RESERVE = 0,
> >>+	CPU,
> >>+	GPU,
> >>+	SENSOR_ID_END,
> >>+};
> >>+
> >>+struct rockchip_thermal_data {
> >>+	const struct rockchip_tsadc_platform_data *pdata;
> >>+	struct thermal_zone_device *tz[SENSOR_ID_END];
> >>+	struct thermal_cooling_device *cdev;
> >>+	void __iomem *regs;
> >>+
> >>+	unsigned long temp_passive;
> >>+	unsigned long hw_shut_temp;
> >>+	unsigned long alarm_temp;
> >>+	bool irq_enabled;
> >>+	int irq;
> >>+	int reset_mode;
> >>+	int chn;
> >>+
> >>+	struct clk *clk;
> >>+	struct clk *pclk;
> >>+};
> >>+
> >>+struct rockchip_tsadc_platform_data {
> >>+	unsigned long temp_passive;
> >>+	unsigned long hw_shut_temp;
> >>+	int reset_mode;
> >>+
> >>+	int (*irq_handle)(void __iomem *reg);
> >Why does it return int and not void? What is the return value of this?
> 
> Fixed. instead of using void.
> >>+	int (*initialize)(int reset_mode, int chn, void __iomem *reg,
> >Why does it return int and not void? What is the return value of this?> +			  unsigned long hw_shut_temp);
> 
> Ditto.
> 
> >>+	int (*control)(void __iomem *reg, bool on);
> >>+	int (*code_to_temp)(u32 code);
> >>+	u32 (*temp_to_code)(int temp);
> >>+	void (*set_alarm_temp)(int chn, void __iomem *reg,
> >>+			       unsigned long alarm_temp);
> >>+};
> >>+
> >>+/* TSADC V2 Sensor info define: */
> >>+#define TSADCV2_AUTO_CON			0x04
> >>+#define TSADCV2_INT_EN				0x08
> >>+#define TSADCV2_INT_PD				0x0c
> >>+#define TSADCV2_DATA(chn)			(0x20+chn*0x04)
> >Unsafe macros. You want to enclose argument in parenthesis.
> 
> Yeah.
> Hmmm, there is no thought of a good way to deal with it at the moment.
> 
> Maybe do that as the follows,but should be a better way to deal.

No, I meant

#define TSADCV2_DATA(chn)	(0x20 + (chn) * 0x04)

This way one can safely pass expressions as argument, like
TSADCV2_DATA(i + 1) will evaluate properly.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-10-11  6:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-10 10:19 [PATCH v8 0/5] Rockchip soc thermal driver Caesar Wang
2014-10-10 10:19 ` Caesar Wang
2014-10-10 10:19 ` [PATCH v8 1/5] thermal: rockchip: add driver for thermal Caesar Wang
2014-10-10 10:19   ` Caesar Wang
2014-10-10 17:46   ` Dmitry Torokhov
2014-10-10 17:46     ` Dmitry Torokhov
2014-10-11  6:01     ` Caesar Wang
2014-10-11  6:01       ` Caesar Wang
2014-10-11  6:20       ` Dmitry Torokhov
2014-10-11  6:20         ` Dmitry Torokhov
2014-10-10 10:19 ` [PATCH v8 2/5] dt-bindings: document Rockchip thermal Caesar Wang
2014-10-10 10:19   ` Caesar Wang
2014-10-10 10:19 ` [PATCH v8 3/5] ARM: dts: add RK3288 Thermal data Caesar Wang
2014-10-10 10:19   ` Caesar Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.