[PATCH v5 0/3] 96boards: add thermal senor support to hikey board

[PATCH v5 0/3] 96boards: add thermal senor support to hikey board

[Xinwei Kong]

From: kongxinwei <kong.kongxinwei@hisilicon.com>

The Linaro connect introduce 96boards series in Hong Kong,The HiKey board
is the first board to be certified 96Boards Consumer Edition compatible.
This board is based on the HiSilicon SoC. you can get more information
from https://www.96boards.org.

The hisilicon SoC contains thermal module, this thermal module has 4 sensors,

	- sensor 0: local sensor;
	- sensor 1: remote sensor for ACPU cluster 1;
	- sensor 2: remote sensor for ACPU cluster 2;
	- sensor 3: remote sensor for GPU;

It can obtain this device temperature by operating this hardware. The new
sensor driver satisfies thermal framework and to realize the ACPU ,GPU and
so on to cool function.

Changes v4->v5;
* Rebase to kernel 4.1-rc1
* Add the DTS file according to the maintainer advice
* Clean up "include" header file and fix detail format
* Deal with negative temperature and add mutex lock to
  protect binding irq variable
* Remove this "const struct themal_trip" from "struct
  hisi_thermal_sensor", making it as local variable
* Make sensor_temp part of "struct hisi_thremal_sensors"

Changes v3->v4;
* using "usleep_range" function instead of "msleep" function
* delete code detail error

Changes v2->v3;
* delete sensor id property in binding document 
* fix sensor driver to satisfy sensor register after deleting sensor id
  property

Changes v1->v2;
* change patch's position between binding document and driver file
* clean up some regiser for enabling thermal sensor
* use mutex lock to replace the spin lock

Changes v0->v1;
* Delete this hi6220 dtsi.
* Fix documentation and error checks.
* Modify this driver which makes use of kernel to decide how to dynamically
  bind the interrupt to hottest sensor.
* Delete "sensor-thres-temp" property and read thermal_zone trips points
  replace of it.
* Delete "sensor-reset-temp" property and define the fixed value replace
  of it.

kongxinwei (3):
  dt-bindings: Document the hi6220 thermal sensor bindings
  thermal: hisilicon: add new hisilicon thermal sensor driver
  dts: hi6220: enable thermal sensor for hisilicon SoC

 .../bindings/thermal/hisilicon-thermal.txt         |  23 ++
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi          | 127 +++++++
 drivers/thermal/Kconfig                            |   8 +
 drivers/thermal/Makefile                           |   1 +
 drivers/thermal/hisi_thermal.c                     | 418 +++++++++++++++++++++
 5 files changed, 577 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt
 create mode 100644 arch/arm64/boot/dts/hisilicon/hi6220.dtsi
 create mode 100644 drivers/thermal/hisi_thermal.c

-- 
1.9.1

[PATCH v5 1/3] dt-bindings: Document the hi6220 thermal sensor bindings

[Xinwei Kong]

From: kongxinwei <kong.kongxinwei@hisilicon.com>

This adds documentation of device tree bindings for the
thermal sensor controller of hi6220 SoC.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Signed-off-by: kongxinwei <kong.kongxinwei@hisilicon.com>
---
 .../bindings/thermal/hisilicon-thermal.txt         | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt

diff --git a/Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt b/Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt
new file mode 100644
index 0000000..b2a349f
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt
@@ -0,0 +1,23 @@
+* Temperature Sensor on hisilicon SoCs
+
+** Required properties :
+
+- compatible: "hisilicon,tsensor".
+- reg: physical base address of thermal sensor and length of memory mapped
+  region.
+- interrupt: The interrupt number to the cpu. Defines the interrupt used
+  by SOCTHERM.
+- clock-names: Input clock name, should be 'thermal_clk'.
+- clocks: phandles for clock specified in "clock-names" property.
+- #thermal-sensor-cells: Should be 1. See ./thermal.txt for a description.
+
+Example :
+
+	tsensor: tsensor@0,f7030700 {
+		compatible = "hisilicon,tsensor";
+		reg = <0x0 0xf7030700 0x0 0x1000>;
+		interrupts = <0 7 0x4>;
+		clocks = <&clock_sys HI6220_TSENSOR_CLK>;
+		clock-names = "thermal_clk";
+		#thermal-sensor-cells = <1>;
+	}
-- 
1.9.1

[PATCH v5 2/3] thermal: hisilicon: add new hisilicon thermal sensor driver

[Xinwei Kong]

From: kongxinwei <kong.kongxinwei@hisilicon.com>

This patch adds the support for hisilicon thermal sensor, within
hisilicon SoC. there will register sensors for thermal framework
and use device tree to bind cooling device.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Signed-off-by: kongxinwei <kong.kongxinwei@hisilicon.com>
---
 drivers/thermal/Kconfig        |   8 +
 drivers/thermal/Makefile       |   1 +
 drivers/thermal/hisi_thermal.c | 418 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 427 insertions(+)
 create mode 100644 drivers/thermal/hisi_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index af40db0..81aee01 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -136,6 +136,14 @@ config THERMAL_EMULATION
 	  because userland can easily disable the thermal policy by simply
 	  flooding this sysfs node with low temperature values.
 
+config HISI_THERMAL
+	tristate "Hisilicon thermal driver"
+	depends on ARCH_HISI && CPU_THERMAL && OF
+	help
+	  Enable this to plug hisilicon's thermal sensor driver into the Linux
+	  thermal framework. cpufreq is used as the cooling device to throttle
+	  CPUs when the passive trip is crossed.
+
 config IMX_THERMAL
 	tristate "Temperature sensor driver for Freescale i.MX SoCs"
 	depends on CPU_THERMAL
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index fa0dc48..08ae7ac 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -39,3 +39,4 @@ obj-$(CONFIG_TI_SOC_THERMAL)	+= ti-soc-thermal/
 obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
 obj-$(CONFIG_ST_THERMAL)	+= st/
 obj-$(CONFIG_TEGRA_SOCTHERM)	+= tegra_soctherm.o
+obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
new file mode 100644
index 0000000..e0a466d
--- /dev/null
+++ b/drivers/thermal/hisi_thermal.c
@@ -0,0 +1,418 @@
+/*
+ * Hisilicon thermal sensor driver
+ *
+ * Copyright (c) 2014-2015 Hisilicon Limited.
+ * Copyright (c) 2014-2015 Linaro Limited.
+ *
+ * Xinwei Kong <kong.kongxinwei@hisilicon.com>
+ * Leo Yan <leo.yan@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/cpufreq.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include "thermal_core.h"
+
+#define TEMP0_TH			(0x4)
+#define TEMP0_RST_TH			(0x8)
+#define TEMP0_CFG			(0xC)
+#define TEMP0_EN			(0x10)
+#define TEMP0_INT_EN			(0x14)
+#define TEMP0_INT_CLR			(0x18)
+#define TEMP0_RST_MSK			(0x1C)
+#define TEMP0_VALUE			(0x28)
+
+#define HISI_TEMP_BASE			(-60)
+#define HISI_TEMP_RESET			(100000)
+
+#define HISI_MAX_SENSORS		4
+
+struct hisi_thermal_sensor {
+	struct hisi_thermal_data *thermal;
+	struct thermal_zone_device *tzd;
+
+	long sensor_temp;
+	uint32_t id;
+	uint32_t thres_temp;
+};
+
+struct hisi_thermal_data {
+	struct mutex thermal_lock;    /* protects register data */
+	struct platform_device *pdev;
+	struct clk *clk;
+	struct hisi_thermal_sensor sensors[HISI_MAX_SENSORS];
+
+	int irq, irq_bind_sensor;
+	bool irq_enabled;
+
+	void __iomem *regs;
+};
+
+/* in millicelsius */
+static inline int _step_to_temp(int step)
+{
+	/*
+	 * Every step equals (1 * 200) / 255 celsius, and finally
+	 * need convert to millicelsius.
+	 */
+	return (HISI_TEMP_BASE + (step * 200 / 255)) * 1000;
+}
+
+static inline long _temp_to_step(long temp)
+{
+	return ((temp / 1000 - HISI_TEMP_BASE) * 255 / 200);
+}
+
+static long hisi_thermal_get_sensor_temp(struct hisi_thermal_data *data,
+					 struct hisi_thermal_sensor *sensor)
+{
+	long val;
+
+	mutex_lock(&data->thermal_lock);
+
+	/* disable interrupt */
+	writel(0x0, data->regs + TEMP0_INT_EN);
+	writel(0x1, data->regs + TEMP0_INT_CLR);
+
+	/* disable module firstly */
+	writel(0x0, data->regs + TEMP0_EN);
+
+	/* select sensor id */
+	writel((sensor->id << 12), data->regs + TEMP0_CFG);
+
+	/* enable module */
+	writel(0x1, data->regs + TEMP0_EN);
+
+	usleep_range(3000, 5000);
+
+	val = readl(data->regs + TEMP0_VALUE);
+	val = _step_to_temp(val);
+
+	mutex_unlock(&data->thermal_lock);
+
+	return val;
+}
+
+static void hisi_thermal_enable_bind_irq_sensor
+			(struct hisi_thermal_data *data)
+{
+	struct hisi_thermal_sensor *sensor;
+
+	sensor = &data->sensors[data->irq_bind_sensor];
+
+	mutex_lock(&data->thermal_lock);
+
+	/* setting the hdak time */
+	writel(0x0, data->regs + TEMP0_CFG);
+
+	/* disable module firstly */
+	writel(0x0, data->regs + TEMP0_RST_MSK);
+	writel(0x0, data->regs + TEMP0_EN);
+
+	/* select sensor id */
+	writel((sensor->id << 12), data->regs + TEMP0_CFG);
+
+	/* enable for interrupt */
+	writel(_temp_to_step(sensor->thres_temp) | 0x0FFFFFF00,
+	       data->regs + TEMP0_TH);
+
+	writel(_temp_to_step(HISI_TEMP_RESET), data->regs + TEMP0_RST_TH);
+
+	/* enable module */
+	writel(0x1, data->regs + TEMP0_RST_MSK);
+	writel(0x1, data->regs + TEMP0_EN);
+
+	writel(0x0, data->regs + TEMP0_INT_CLR);
+	writel(0x1, data->regs + TEMP0_INT_EN);
+
+	usleep_range(3000, 5000);
+
+	mutex_unlock(&data->thermal_lock);
+}
+
+static void hisi_thermal_disable_sensor(struct hisi_thermal_data *data)
+{
+	mutex_lock(&data->thermal_lock);
+
+	/* disable sensor module */
+	writel(0x0, data->regs + TEMP0_INT_EN);
+	writel(0x0, data->regs + TEMP0_RST_MSK);
+	writel(0x0, data->regs + TEMP0_EN);
+
+	mutex_unlock(&data->thermal_lock);
+}
+
+static int hisi_thermal_get_temp(void *_sensor, long *temp)
+{
+	struct hisi_thermal_sensor *sensor = _sensor;
+	struct hisi_thermal_data *data = sensor->thermal;
+
+	int sensor_id = 0, i;
+	long max_temp = 0;
+
+	*temp = hisi_thermal_get_sensor_temp(data, sensor);
+
+	sensor->sensor_temp = *temp;
+
+	for (i = 0; i < HISI_MAX_SENSORS; i++) {
+		if (data->sensors[i].sensor_temp >= max_temp) {
+			max_temp = data->sensors[i].sensor_temp;
+			sensor_id = i;
+		}
+	}
+
+	mutex_lock(&data->thermal_lock);
+	data->irq_bind_sensor = sensor_id;
+	mutex_unlock(&data->thermal_lock);
+
+	dev_dbg(&data->pdev->dev, "id=%d, irq=%d, temp=%ld, thres=%d\n",
+		sensor->id, data->irq_enabled, *temp, sensor->thres_temp);
+	/*
+	 * Bind irq to sensor for two cases:
+	 *   Reenable alarm IRQ if temperature below threshold;
+	 *   if irq has been enabled, always set it;
+	 */
+	if (data->irq_enabled) {
+		hisi_thermal_enable_bind_irq_sensor(data);
+		return 0;
+	}
+
+	if (max_temp < sensor->thres_temp) {
+		data->irq_enabled = true;
+		hisi_thermal_enable_bind_irq_sensor(data);
+		enable_irq(data->irq);
+	}
+
+	return 0;
+}
+
+static struct thermal_zone_of_device_ops hisi_of_thermal_ops = {
+	.get_temp = hisi_thermal_get_temp,
+};
+
+static irqreturn_t hisi_thermal_alarm_irq(int irq, void *dev)
+{
+	struct hisi_thermal_data *data = dev;
+
+	disable_irq_nosync(irq);
+	data->irq_enabled = false;
+
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
+{
+	struct hisi_thermal_data *data = dev;
+	struct hisi_thermal_sensor *sensor;
+	int i;
+
+	sensor = &data->sensors[data->irq_bind_sensor];
+
+	dev_crit(&data->pdev->dev, "THERMAL ALARM: T > %d\n",
+		 sensor->thres_temp / 1000);
+
+	for (i = 0; i < HISI_MAX_SENSORS; i++)
+		thermal_zone_device_update(data->sensors[i].tzd);
+
+	return IRQ_HANDLED;
+}
+
+static int hisi_thermal_register_sensor(struct platform_device *pdev,
+					struct hisi_thermal_data *data,
+					struct hisi_thermal_sensor *sensor,
+					int index)
+{
+	int ret, i;
+	const struct thermal_trip *trip;
+
+	sensor->id = index;
+	sensor->thermal = data;
+
+	sensor->tzd = thermal_zone_of_sensor_register(&pdev->dev, sensor->id,
+				sensor, &hisi_of_thermal_ops);
+	if (IS_ERR(sensor->tzd)) {
+		ret = PTR_ERR(sensor->tzd);
+		dev_err(&pdev->dev, "failed to register sensor id %d: %d\n",
+			sensor->id, ret);
+		return ret;
+	}
+
+	trip = of_thermal_get_trip_points(sensor->tzd);
+
+	for (i = 0; i < of_thermal_get_ntrips(sensor->tzd); i++) {
+		if (trip[i].type == THERMAL_TRIP_PASSIVE) {
+			sensor->thres_temp = trip[i].temperature;
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id of_hisi_thermal_match[] = {
+	{ .compatible = "hisilicon,tsensor" },
+	{ /* end */ }
+};
+MODULE_DEVICE_TABLE(of, of_hisi_thermal_match);
+
+static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
+				       bool on)
+{
+	struct thermal_zone_device *tzd = sensor->tzd;
+
+	tzd->ops->set_mode(tzd,
+		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
+}
+
+static int hisi_thermal_probe(struct platform_device *pdev)
+{
+	struct hisi_thermal_data *data;
+	struct resource *res;
+	int i;
+	int ret;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	mutex_init(&data->thermal_lock);
+	data->pdev = pdev;
+
+	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, "failed to get io address\n");
+		return PTR_ERR(data->regs);
+	}
+
+	data->irq = platform_get_irq(pdev, 0);
+	if (data->irq < 0)
+		return data->irq;
+
+	ret = devm_request_threaded_irq(&pdev->dev, data->irq,
+					hisi_thermal_alarm_irq,
+					hisi_thermal_alarm_irq_thread,
+					0, "hisi_thermal", data);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to request alarm irq: %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, data);
+
+	data->clk = devm_clk_get(&pdev->dev, "thermal_clk");
+	if (IS_ERR(data->clk)) {
+		ret = PTR_ERR(data->clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
+				"failed to get thermal clk: %d\n", ret);
+		return ret;
+	}
+
+	/* enable clock for thermal */
+	ret = clk_prepare_enable(data->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
+		return ret;
+	}
+
+	for (i = 0; i < HISI_MAX_SENSORS; ++i) {
+		ret = hisi_thermal_register_sensor(pdev, data,
+						   &data->sensors[i], i);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"failed to register thermal sensor: %d\n", ret);
+			goto err_get_sensor_data;
+		}
+	}
+
+	hisi_thermal_enable_bind_irq_sensor(data);
+	data->irq_enabled = true;
+
+	for (i = 0; i < HISI_MAX_SENSORS; i++)
+		hisi_thermal_toggle_sensor(&data->sensors[i], true);
+
+	return 0;
+
+err_get_sensor_data:
+	clk_disable_unprepare(data->clk);
+
+	return ret;
+}
+
+static int hisi_thermal_remove(struct platform_device *pdev)
+{
+	struct hisi_thermal_data *data = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < HISI_MAX_SENSORS; i++) {
+		struct hisi_thermal_sensor *sensor = &data->sensors[i];
+
+		hisi_thermal_toggle_sensor(sensor, false);
+		thermal_zone_of_sensor_unregister(&pdev->dev, sensor->tzd);
+	}
+
+	hisi_thermal_disable_sensor(data);
+	clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int hisi_thermal_suspend(struct device *dev)
+{
+	struct hisi_thermal_data *data = dev_get_drvdata(dev);
+
+	hisi_thermal_disable_sensor(data);
+	data->irq_enabled = false;
+
+	clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+static int hisi_thermal_resume(struct device *dev)
+{
+	struct hisi_thermal_data *data = dev_get_drvdata(dev);
+
+	clk_prepare_enable(data->clk);
+
+	data->irq_enabled = true;
+	hisi_thermal_enable_bind_irq_sensor(data);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(hisi_thermal_pm_ops,
+			 hisi_thermal_suspend, hisi_thermal_resume);
+
+static struct platform_driver hisi_thermal_driver = {
+	.driver = {
+		.name		= "hisi_thermal",
+		.owner		= THIS_MODULE,
+		.pm		= &hisi_thermal_pm_ops,
+		.of_match_table = of_hisi_thermal_match,
+	},
+	.probe	= hisi_thermal_probe,
+	.remove	= hisi_thermal_remove,
+};
+
+module_platform_driver(hisi_thermal_driver);
+
+MODULE_AUTHOR("Xinwei Kong <kong.kongxinwei@hisilicon.com>");
+MODULE_AUTHOR("Leo Yan <leo.yan@linaro.org>");
+MODULE_DESCRIPTION("Hisilicon thermal driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH v5 3/3] dts: hi6220: enable thermal sensor for hisilicon SoC

[Xinwei Kong]

From: kongxinwei <kong.kongxinwei@hisilicon.com>

Dts includes two part: the first part is related with thermal sensor;
the second part is related with thermal zones, in this part it will
define the thermal zones and which sensor device should be bound to.
it also need specify the polling interval for every thermal zone.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Signed-off-by: kongxinwei <kong.kongxinwei@hisilicon.com>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 127 ++++++++++++++++++++++++++++++
 1 file changed, 127 insertions(+)
 create mode 100644 arch/arm64/boot/dts/hisilicon/hi6220.dtsi

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
new file mode 100644
index 0000000..82f213d
--- /dev/null
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -0,0 +1,127 @@
+
+#include <dt-bindings/thermal/thermal.h>
+
+/ {
+
+	tsensor: tsensor@0,f7030700 {
+		compatible = "hisilicon,tsensor";
+		reg = <0x0 0xf7030700 0x0 0x1000>;
+		interrupts = <0 7 0x4>;
+		clocks = <&clock_sys HI6220_TSENSOR_CLK>;
+		clock-names = "thermal_clk";
+		#thermal-sensor-cells = <1>;
+
+	thermal-zones {
+		local: local {
+			/* milliseconds */
+			polling-delay-passive = <1000>;
+			/* milliseconds */
+			polling-delay = <5000>;
+
+			/* sensor	ID */
+			thermal-sensors = <&tsensor  0>;
+
+			trips {
+				local_alert: local_alert {
+					/* millicelsius */
+					temperature = <70000>;
+					/* millicelsius */
+					hysteresis = <2000>;
+					type = "passive";
+				};
+				local_crit: local_crit {
+					temperature = <90000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				/* There are currently no cooling maps
+				because there are no cooling devices */
+			};
+		};
+
+		cluster1: cluster1 {
+			polling-delay-passive = <1000>;
+			polling-delay = <5000>;
+
+			/* sensor	ID */
+			thermal-sensors = <&tsensor  1>;
+
+			trips {
+				cluster1_alert: cluster1_alert {
+					temperature = <70000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+				cluster1_crit: cluster1_crit {
+					temperature = <90000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				/* There are currently no cooling maps
+				because there are no cooling devices */
+			};
+		};
+
+		cluster0: cluster0 {
+			polling-delay-passive = <1000>;
+			polling-delay = <5000>;
+
+			/* sensor	ID */
+			thermal-sensors = <&tsensor  2>;
+
+			trips {
+				cluster0_alert: cluster0_alert {
+					temperature = <70000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+				cluster0_crit: cluster0_crit {
+					temperature = <90000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				map0 {
+					trip = <&cluster0_alert>;
+					cooling-device =
+					    <&cpu0 THERMAL_NO_LIMIT
+						THERMAL_NO_LIMIT>;
+				};
+			};
+		};
+
+		gpu: gpu {
+			polling-delay-passive = <1000>;
+			polling-delay = <5000>;
+
+			/* sensor	ID */
+			thermal-sensors = <&tsensor  3>;
+
+			trips {
+				gpu_alert: gpu_alert {
+					temperature = <70000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+				gpu_crit: gpu_crit {
+					temperature = <90000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				/* There are currently no cooling maps
+				because there are no cooling devices */
+			};
+		};
+	};
+}
-- 
1.9.1

Re: [PATCH v5 1/3] dt-bindings: Document the hi6220 thermal sensor bindings

[Leo Yan]

On Mon, May 18, 2015 at 03:11:47PM +0800, Xinwei Kong wrote:
> From: kongxinwei <kong.kongxinwei@hisilicon.com>
> 
> This adds documentation of device tree bindings for the
> thermal sensor controller of hi6220 SoC.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: kongxinwei <kong.kongxinwei@hisilicon.com>
> ---
>  .../bindings/thermal/hisilicon-thermal.txt         | 23 ++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt
> 
> diff --git a/Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt b/Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt
> new file mode 100644
> index 0000000..b2a349f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt
> @@ -0,0 +1,23 @@
> +* Temperature Sensor on hisilicon SoCs
> +
> +** Required properties :
> +
> +- compatible: "hisilicon,tsensor".
> +- reg: physical base address of thermal sensor and length of memory mapped
> +  region.
> +- interrupt: The interrupt number to the cpu. Defines the interrupt used
> +  by SOCTHERM.

s/SOCTHERM/tsensor/

> +- clock-names: Input clock name, should be 'thermal_clk'.
> +- clocks: phandles for clock specified in "clock-names" property.
> +- #thermal-sensor-cells: Should be 1. See ./thermal.txt for a description.
> +
> +Example :
> +
> +	tsensor: tsensor@0,f7030700 {
> +		compatible = "hisilicon,tsensor";
> +		reg = <0x0 0xf7030700 0x0 0x1000>;
> +		interrupts = <0 7 0x4>;
> +		clocks = <&clock_sys HI6220_TSENSOR_CLK>;
> +		clock-names = "thermal_clk";
> +		#thermal-sensor-cells = <1>;
> +	}
> -- 
> 1.9.1
> 
> 

Re: [PATCH v5 3/3] dts: hi6220: enable thermal sensor for hisilicon SoC

[Xinwei Kong]

On 2015/5/20 12:51, Eduardo Valentin wrote:
> On Mon, May 18, 2015 at 03:11:49PM +0800, Xinwei Kong wrote:
>> From: kongxinwei <kong.kongxinwei@hisilicon.com>
>>
>> Dts includes two part: the first part is related with thermal sensor;
>> the second part is related with thermal zones, in this part it will
>> define the thermal zones and which sensor device should be bound to.
>> it also need specify the polling interval for every thermal zone.
>>
>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
>> Signed-off-by: kongxinwei <kong.kongxinwei@hisilicon.com>
>> ---
>>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 127 ++++++++++++++++++++++++++++++
>>  1 file changed, 127 insertions(+)
>>  create mode 100644 arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> new file mode 100644
>> index 0000000..82f213d
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> @@ -0,0 +1,127 @@
>> +
>> +#include <dt-bindings/thermal/thermal.h>
>> +
>> +/ {
>> +
>> +	tsensor: tsensor@0,f7030700 {
>> +		compatible = "hisilicon,tsensor";
>> +		reg = <0x0 0xf7030700 0x0 0x1000>;
>> +		interrupts = <0 7 0x4>;
>> +		clocks = <&clock_sys HI6220_TSENSOR_CLK>;
>> +		clock-names = "thermal_clk";
>> +		#thermal-sensor-cells = <1>;

 		}

oh, my god. I miss it. add "}".
>> +
>> +	thermal-zones {
> 
> The thermal-zones node is typically in the root node, not inside the
> sensor node.
> 
>> +		local: local {
>> +			/* milliseconds */
>> +			polling-delay-passive = <1000>;
>> +			/* milliseconds */
>> +			polling-delay = <5000>;
>> +
>> +			/* sensor	ID */
>> +			thermal-sensors = <&tsensor  0>;
>> +
>> +			trips {
>> +				local_alert: local_alert {
>> +					/* millicelsius */
>> +					temperature = <70000>;
>> +					/* millicelsius */
>> +					hysteresis = <2000>;
>> +					type = "passive";
>> +				};
>> +				local_crit: local_crit {
>> +					temperature = <90000>;
>> +					hysteresis = <2000>;
>> +					type = "critical";
>> +				};
>> +			};
>> +
>> +			cooling-maps {
>> +				/* There are currently no cooling maps
>> +				because there are no cooling devices */
> 
> Not even cpufreq? in your driver you mention about cpufreq cooling.
> 
> Do you have cpufreq-dt driver  properly setup in your board?
> 
>> +			};
>> +		};
>> +
>> +		cluster1: cluster1 {
>> +			polling-delay-passive = <1000>;
>> +			polling-delay = <5000>;
>> +
>> +			/* sensor	ID */
>> +			thermal-sensors = <&tsensor  1>;
>> +
>> +			trips {
>> +				cluster1_alert: cluster1_alert {
>> +					temperature = <70000>;
>> +					hysteresis = <2000>;
>> +					type = "passive";
>> +				};
>> +				cluster1_crit: cluster1_crit {
>> +					temperature = <90000>;
>> +					hysteresis = <2000>;
>> +					type = "critical";
>> +				};
>> +			};
>> +
>> +			cooling-maps {
>> +				/* There are currently no cooling maps
>> +				because there are no cooling devices */
>> +			};
>> +		};
>> +
>> +		cluster0: cluster0 {
>> +			polling-delay-passive = <1000>;
>> +			polling-delay = <5000>;
>> +
>> +			/* sensor	ID */
>> +			thermal-sensors = <&tsensor  2>;
>> +
>> +			trips {
>> +				cluster0_alert: cluster0_alert {
>> +					temperature = <70000>;
>> +					hysteresis = <2000>;
>> +					type = "passive";
>> +				};
>> +				cluster0_crit: cluster0_crit {
>> +					temperature = <90000>;
>> +					hysteresis = <2000>;
>> +					type = "critical";
>> +				};
>> +			};
>> +
>> +			cooling-maps {
>> +				map0 {
>> +					trip = <&cluster0_alert>;
>> +					cooling-device =
>> +					    <&cpu0 THERMAL_NO_LIMIT
>> +						THERMAL_NO_LIMIT>;
>> +				};
>> +			};
>> +		};
>> +
>> +		gpu: gpu {
>> +			polling-delay-passive = <1000>;
>> +			polling-delay = <5000>;
>> +
>> +			/* sensor	ID */
>> +			thermal-sensors = <&tsensor  3>;
>> +
>> +			trips {
>> +				gpu_alert: gpu_alert {
>> +					temperature = <70000>;
>> +					hysteresis = <2000>;
>> +					type = "passive";
>> +				};
>> +				gpu_crit: gpu_crit {
>> +					temperature = <90000>;
>> +					hysteresis = <2000>;
>> +					type = "critical";
>> +				};
>> +			};
>> +
>> +			cooling-maps {
>> +				/* There are currently no cooling maps
>> +				because there are no cooling devices */
>> +			};
>> +		};
>> +	};
>> +}
> 
> Are you sure this DT file works? I think it misses a closing \}.

I will re-varify  this driver and enable or use this tsensor based
mainline kernel  4.1-rc1.
> 
>> -- 
>> 1.9.1
>>
>>

Re: [PATCH v5 3/3] dts: hi6220: enable thermal sensor for hisilicon SoC

[Leo Yan]

On Tue, May 19, 2015 at 09:51:01PM -0700, Eduardo Valentin wrote:
> On Mon, May 18, 2015 at 03:11:49PM +0800, Xinwei Kong wrote:
> > From: kongxinwei <kong.kongxinwei@hisilicon.com>
> > 
> > Dts includes two part: the first part is related with thermal sensor;
> > the second part is related with thermal zones, in this part it will
> > define the thermal zones and which sensor device should be bound to.
> > it also need specify the polling interval for every thermal zone.
> > 
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > Signed-off-by: kongxinwei <kong.kongxinwei@hisilicon.com>
> > ---
> >  arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 127 ++++++++++++++++++++++++++++++
> >  1 file changed, 127 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> > new file mode 100644
> > index 0000000..82f213d
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> > @@ -0,0 +1,127 @@
> > +
> > +#include <dt-bindings/thermal/thermal.h>
> > +
> > +/ {
> > +
> > +	tsensor: tsensor@0,f7030700 {
> > +		compatible = "hisilicon,tsensor";
> > +		reg = <0x0 0xf7030700 0x0 0x1000>;
> > +		interrupts = <0 7 0x4>;
> > +		clocks = <&clock_sys HI6220_TSENSOR_CLK>;
> > +		clock-names = "thermal_clk";
> > +		#thermal-sensor-cells = <1>;
> > +
> > +	thermal-zones {
> 
> The thermal-zones node is typically in the root node, not inside the
> sensor node.
> 
> > +		local: local {
> > +			/* milliseconds */
> > +			polling-delay-passive = <1000>;
> > +			/* milliseconds */
> > +			polling-delay = <5000>;
> > +
> > +			/* sensor	ID */
> > +			thermal-sensors = <&tsensor  0>;
> > +
> > +			trips {
> > +				local_alert: local_alert {
> > +					/* millicelsius */
> > +					temperature = <70000>;
> > +					/* millicelsius */
> > +					hysteresis = <2000>;
> > +					type = "passive";
> > +				};
> > +				local_crit: local_crit {
> > +					temperature = <90000>;
> > +					hysteresis = <2000>;
> > +					type = "critical";
> > +				};
> > +			};
> > +
> > +			cooling-maps {
> > +				/* There are currently no cooling maps
> > +				because there are no cooling devices */
> 
> Not even cpufreq? in your driver you mention about cpufreq cooling.
> 
> Do you have cpufreq-dt driver  properly setup in your board?

Yes, cpufreq-dt will be used on hi6220 to enable cpufreq cooling
device. [1]

Xinwei will resend this patch, and should verify it based on
hi6220's minimal system patch series. [2]

[1] http://archive.arm.linux.org.uk/lurker/message/20150330.052637.16e0f3f4.en.html
[2] http://archive.arm.linux.org.uk/lurker/message/20150507.140019.bde047ed.en.html

Thanks,
Leo Yan

Re: [PATCH v5 2/3] thermal: hisilicon: add new hisilicon thermal sensor driver

[Xinwei Kong]

On 2015/5/20 12:46, Eduardo Valentin wrote:
> On Mon, May 18, 2015 at 03:11:48PM +0800, Xinwei Kong wrote:
>> From: kongxinwei <kong.kongxinwei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>>
>> This patch adds the support for hisilicon thermal sensor, within
>> hisilicon SoC. there will register sensors for thermal framework
>> and use device tree to bind cooling device.
>>
>> Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Signed-off-by: kongxinwei <kong.kongxinwei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>> ---
>>  drivers/thermal/Kconfig        |   8 +
>>  drivers/thermal/Makefile       |   1 +
>>  drivers/thermal/hisi_thermal.c | 418 +++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 427 insertions(+)
>>  create mode 100644 drivers/thermal/hisi_thermal.c
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index af40db0..81aee01 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -136,6 +136,14 @@ config THERMAL_EMULATION
>>  	  because userland can easily disable the thermal policy by simply
>>  	  flooding this sysfs node with low temperature values.
>>  
>> +config HISI_THERMAL
>> +	tristate "Hisilicon thermal driver"
>> +	depends on ARCH_HISI && CPU_THERMAL && OF
>> +	help
>> +	  Enable this to plug hisilicon's thermal sensor driver into the Linux
>> +	  thermal framework. cpufreq is used as the cooling device to throttle
>> +	  CPUs when the passive trip is crossed.
>> +
>>  config IMX_THERMAL
>>  	tristate "Temperature sensor driver for Freescale i.MX SoCs"
>>  	depends on CPU_THERMAL
>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>> index fa0dc48..08ae7ac 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -39,3 +39,4 @@ obj-$(CONFIG_TI_SOC_THERMAL)	+= ti-soc-thermal/
>>  obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
>>  obj-$(CONFIG_ST_THERMAL)	+= st/
>>  obj-$(CONFIG_TEGRA_SOCTHERM)	+= tegra_soctherm.o
>> +obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
>> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
>> new file mode 100644
>> index 0000000..e0a466d
>> --- /dev/null
>> +++ b/drivers/thermal/hisi_thermal.c
>> @@ -0,0 +1,418 @@
>> +/*
>> + * Hisilicon thermal sensor driver
>> + *
>> + * Copyright (c) 2014-2015 Hisilicon Limited.
>> + * Copyright (c) 2014-2015 Linaro Limited.
>> + *
>> + * Xinwei Kong <kong.kongxinwei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>> + * Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/cpufreq.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +
> 
> 
> I guess you trimmed too much :-)
> 
> This code does not compile:
> drivers/thermal/hisi_thermal.c: In function
> ‘hisi_thermal_get_sensor_temp’:
> drivers/thermal/hisi_thermal.c:86:2: error: implicit declaration of
> function ‘writel’ [-Werror=implicit-function-declaration]
>   writel(0x0, data->regs + TEMP0_INT_EN);
>     ^
> drivers/thermal/hisi_thermal.c:100:2: error: implicit declaration of
> function ‘readl’ [-Werror=implicit-function-declaration]
>       val = readl(data->regs + TEMP0_VALUE);
>         ^
> cc1: some warnings being treated as errors
> 
> 
> Please add:
> #include <linux/io.h>
> 
I try it. Thank you.

> 
>> +#include "thermal_core.h"
>> +
>> +#define TEMP0_TH			(0x4)
>> +#define TEMP0_RST_TH			(0x8)
>> +#define TEMP0_CFG			(0xC)
>> +#define TEMP0_EN			(0x10)
>> +#define TEMP0_INT_EN			(0x14)
>> +#define TEMP0_INT_CLR			(0x18)
>> +#define TEMP0_RST_MSK			(0x1C)
>> +#define TEMP0_VALUE			(0x28)
>> +
>> +#define HISI_TEMP_BASE			(-60)
>> +#define HISI_TEMP_RESET			(100000)
>> +
>> +#define HISI_MAX_SENSORS		4
>> +
>> +struct hisi_thermal_sensor {
>> +	struct hisi_thermal_data *thermal;
>> +	struct thermal_zone_device *tzd;
>> +
>> +	long sensor_temp;
>> +	uint32_t id;
>> +	uint32_t thres_temp;
>> +};
>> +
>> +struct hisi_thermal_data {
>> +	struct mutex thermal_lock;    /* protects register data */
>> +	struct platform_device *pdev;
>> +	struct clk *clk;
>> +	struct hisi_thermal_sensor sensors[HISI_MAX_SENSORS];
>> +
>> +	int irq, irq_bind_sensor;
>> +	bool irq_enabled;
>> +
>> +	void __iomem *regs;
>> +};
>> +
>> +/* in millicelsius */
>> +static inline int _step_to_temp(int step)
>> +{
>> +	/*
>> +	 * Every step equals (1 * 200) / 255 celsius, and finally
>> +	 * need convert to millicelsius.
>> +	 */
>> +	return (HISI_TEMP_BASE + (step * 200 / 255)) * 1000;
>> +}
>> +
>> +static inline long _temp_to_step(long temp)
>> +{
>> +	return ((temp / 1000 - HISI_TEMP_BASE) * 255 / 200);
>> +}
>> +
>> +static long hisi_thermal_get_sensor_temp(struct hisi_thermal_data *data,
>> +					 struct hisi_thermal_sensor *sensor)
>> +{
>> +	long val;
>> +
>> +	mutex_lock(&data->thermal_lock);
>> +
>> +	/* disable interrupt */
>> +	writel(0x0, data->regs + TEMP0_INT_EN);
>> +	writel(0x1, data->regs + TEMP0_INT_CLR);
>> +
>> +	/* disable module firstly */
>> +	writel(0x0, data->regs + TEMP0_EN);
>> +
>> +	/* select sensor id */
>> +	writel((sensor->id << 12), data->regs + TEMP0_CFG);
>> +
>> +	/* enable module */
>> +	writel(0x1, data->regs + TEMP0_EN);
>> +
>> +	usleep_range(3000, 5000);
>> +
>> +	val = readl(data->regs + TEMP0_VALUE);
>> +	val = _step_to_temp(val);
>> +
>> +	mutex_unlock(&data->thermal_lock);
>> +
>> +	return val;
>> +}
>> +
>> +static void hisi_thermal_enable_bind_irq_sensor
>> +			(struct hisi_thermal_data *data)
>> +{
>> +	struct hisi_thermal_sensor *sensor;
>> +
>> +	sensor = &data->sensors[data->irq_bind_sensor];
> 
> This reader of irq_bind_sensor needs to be locked. So, move this line
> below the mutex_lock below:
> 
>> +
>> +	mutex_lock(&data->thermal_lock);
>> +
> 
> <here>
> 
 Ok, fix it in the next version. THX

>> +	/* setting the hdak time */
>> +	writel(0x0, data->regs + TEMP0_CFG);
>> +
>> +	/* disable module firstly */
>> +	writel(0x0, data->regs + TEMP0_RST_MSK);
>> +	writel(0x0, data->regs + TEMP0_EN);
>> +
>> +	/* select sensor id */
>> +	writel((sensor->id << 12), data->regs + TEMP0_CFG);
>> +
>> +	/* enable for interrupt */
>> +	writel(_temp_to_step(sensor->thres_temp) | 0x0FFFFFF00,
>> +	       data->regs + TEMP0_TH);
>> +
>> +	writel(_temp_to_step(HISI_TEMP_RESET), data->regs + TEMP0_RST_TH);
>> +
>> +	/* enable module */
>> +	writel(0x1, data->regs + TEMP0_RST_MSK);
>> +	writel(0x1, data->regs + TEMP0_EN);
>> +
>> +	writel(0x0, data->regs + TEMP0_INT_CLR);
>> +	writel(0x1, data->regs + TEMP0_INT_EN);
>> +
>> +	usleep_range(3000, 5000);
>> +
>> +	mutex_unlock(&data->thermal_lock);
>> +}
>> +
>> +static void hisi_thermal_disable_sensor(struct hisi_thermal_data *data)
>> +{
>> +	mutex_lock(&data->thermal_lock);
>> +
>> +	/* disable sensor module */
>> +	writel(0x0, data->regs + TEMP0_INT_EN);
>> +	writel(0x0, data->regs + TEMP0_RST_MSK);
>> +	writel(0x0, data->regs + TEMP0_EN);
>> +
>> +	mutex_unlock(&data->thermal_lock);
>> +}
>> +
>> +static int hisi_thermal_get_temp(void *_sensor, long *temp)
>> +{
>> +	struct hisi_thermal_sensor *sensor = _sensor;
>> +	struct hisi_thermal_data *data = sensor->thermal;
>> +
>> +	int sensor_id = 0, i;
>> +	long max_temp = 0;
>> +
>> +	*temp = hisi_thermal_get_sensor_temp(data, sensor);
>> +
>> +	sensor->sensor_temp = *temp;
>> +
>> +	for (i = 0; i < HISI_MAX_SENSORS; i++) {
>> +		if (data->sensors[i].sensor_temp >= max_temp) {
>> +			max_temp = data->sensors[i].sensor_temp;
>> +			sensor_id = i;
>> +		}
>> +	}
>> +
>> +	mutex_lock(&data->thermal_lock);
>> +	data->irq_bind_sensor = sensor_id;
>> +	mutex_unlock(&data->thermal_lock);
>> +
>> +	dev_dbg(&data->pdev->dev, "id=%d, irq=%d, temp=%ld, thres=%d\n",
>> +		sensor->id, data->irq_enabled, *temp, sensor->thres_temp);
>> +	/*
>> +	 * Bind irq to sensor for two cases:
>> +	 *   Reenable alarm IRQ if temperature below threshold;
>> +	 *   if irq has been enabled, always set it;
>> +	 */
>> +	if (data->irq_enabled) {
>> +		hisi_thermal_enable_bind_irq_sensor(data);
>> +		return 0;
>> +	}
>> +
>> +	if (max_temp < sensor->thres_temp) {
>> +		data->irq_enabled = true;
>> +		hisi_thermal_enable_bind_irq_sensor(data);
>> +		enable_irq(data->irq);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static struct thermal_zone_of_device_ops hisi_of_thermal_ops = {
>> +	.get_temp = hisi_thermal_get_temp,
>> +};
>> +
>> +static irqreturn_t hisi_thermal_alarm_irq(int irq, void *dev)
>> +{
>> +	struct hisi_thermal_data *data = dev;
>> +
>> +	disable_irq_nosync(irq);
>> +	data->irq_enabled = false;
>> +
>> +	return IRQ_WAKE_THREAD;
>> +}
>> +
>> +static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
>> +{
>> +	struct hisi_thermal_data *data = dev;
>> +	struct hisi_thermal_sensor *sensor;
>> +	int i;
>> +
>> +	sensor = &data->sensors[data->irq_bind_sensor];
>> +
> 
> this reader needs to be locked too.
> 
>> +	dev_crit(&data->pdev->dev, "THERMAL ALARM: T > %d\n",
>> +		 sensor->thres_temp / 1000);
>> +
>> +	for (i = 0; i < HISI_MAX_SENSORS; i++)
>> +		thermal_zone_device_update(data->sensors[i].tzd);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int hisi_thermal_register_sensor(struct platform_device *pdev,
>> +					struct hisi_thermal_data *data,
>> +					struct hisi_thermal_sensor *sensor,
>> +					int index)
>> +{
>> +	int ret, i;
>> +	const struct thermal_trip *trip;
>> +
>> +	sensor->id = index;
>> +	sensor->thermal = data;
>> +
>> +	sensor->tzd = thermal_zone_of_sensor_register(&pdev->dev, sensor->id,
>> +				sensor, &hisi_of_thermal_ops);
>> +	if (IS_ERR(sensor->tzd)) {
>> +		ret = PTR_ERR(sensor->tzd);
>> +		dev_err(&pdev->dev, "failed to register sensor id %d: %d\n",
>> +			sensor->id, ret);
>> +		return ret;
>> +	}
>> +
>> +	trip = of_thermal_get_trip_points(sensor->tzd);
>> +
>> +	for (i = 0; i < of_thermal_get_ntrips(sensor->tzd); i++) {
>> +		if (trip[i].type == THERMAL_TRIP_PASSIVE) {
>> +			sensor->thres_temp = trip[i].temperature;
>> +			break;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id of_hisi_thermal_match[] = {
>> +	{ .compatible = "hisilicon,tsensor" },
>> +	{ /* end */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, of_hisi_thermal_match);
>> +
>> +static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
>> +				       bool on)
>> +{
>> +	struct thermal_zone_device *tzd = sensor->tzd;
>> +
>> +	tzd->ops->set_mode(tzd,
>> +		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
>> +}
>> +
>> +static int hisi_thermal_probe(struct platform_device *pdev)
>> +{
>> +	struct hisi_thermal_data *data;
>> +	struct resource *res;
>> +	int i;
>> +	int ret;
>> +
>> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	mutex_init(&data->thermal_lock);
>> +	data->pdev = pdev;
>> +
>> +	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, "failed to get io address\n");
>> +		return PTR_ERR(data->regs);
>> +	}
>> +
>> +	data->irq = platform_get_irq(pdev, 0);
>> +	if (data->irq < 0)
>> +		return data->irq;
>> +
>> +	ret = devm_request_threaded_irq(&pdev->dev, data->irq,
>> +					hisi_thermal_alarm_irq,
>> +					hisi_thermal_alarm_irq_thread,
>> +					0, "hisi_thermal", data);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "failed to request alarm irq: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	platform_set_drvdata(pdev, data);
>> +
>> +	data->clk = devm_clk_get(&pdev->dev, "thermal_clk");
>> +	if (IS_ERR(data->clk)) {
>> +		ret = PTR_ERR(data->clk);
>> +		if (ret != -EPROBE_DEFER)
>> +			dev_err(&pdev->dev,
>> +				"failed to get thermal clk: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* enable clock for thermal */
>> +	ret = clk_prepare_enable(data->clk);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	for (i = 0; i < HISI_MAX_SENSORS; ++i) {
>> +		ret = hisi_thermal_register_sensor(pdev, data,
>> +						   &data->sensors[i], i);
>> +		if (ret) {
>> +			dev_err(&pdev->dev,
>> +				"failed to register thermal sensor: %d\n", ret);
>> +			goto err_get_sensor_data;
>> +		}
>> +	}
>> +
>> +	hisi_thermal_enable_bind_irq_sensor(data);
>> +	data->irq_enabled = true;
>> +
>> +	for (i = 0; i < HISI_MAX_SENSORS; i++)
>> +		hisi_thermal_toggle_sensor(&data->sensors[i], true);
>> +
>> +	return 0;
>> +
>> +err_get_sensor_data:
>> +	clk_disable_unprepare(data->clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static int hisi_thermal_remove(struct platform_device *pdev)
>> +{
>> +	struct hisi_thermal_data *data = platform_get_drvdata(pdev);
>> +	int i;
>> +
>> +	for (i = 0; i < HISI_MAX_SENSORS; i++) {
>> +		struct hisi_thermal_sensor *sensor = &data->sensors[i];
>> +
>> +		hisi_thermal_toggle_sensor(sensor, false);
>> +		thermal_zone_of_sensor_unregister(&pdev->dev, sensor->tzd);
>> +	}
>> +
>> +	hisi_thermal_disable_sensor(data);
>> +	clk_disable_unprepare(data->clk);
>> +
>> +	return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int hisi_thermal_suspend(struct device *dev)
>> +{
>> +	struct hisi_thermal_data *data = dev_get_drvdata(dev);
>> +
>> +	hisi_thermal_disable_sensor(data);
>> +	data->irq_enabled = false;
>> +
>> +	clk_disable_unprepare(data->clk);
>> +
>> +	return 0;
>> +}
>> +
>> +static int hisi_thermal_resume(struct device *dev)
>> +{
>> +	struct hisi_thermal_data *data = dev_get_drvdata(dev);
>> +
>> +	clk_prepare_enable(data->clk);
>> +
>> +	data->irq_enabled = true;
>> +	hisi_thermal_enable_bind_irq_sensor(data);
>> +
>> +	return 0;
>> +}
>> +#endif
>> +
>> +static SIMPLE_DEV_PM_OPS(hisi_thermal_pm_ops,
>> +			 hisi_thermal_suspend, hisi_thermal_resume);
>> +
>> +static struct platform_driver hisi_thermal_driver = {
>> +	.driver = {
>> +		.name		= "hisi_thermal",
>> +		.owner		= THIS_MODULE,
>> +		.pm		= &hisi_thermal_pm_ops,
>> +		.of_match_table = of_hisi_thermal_match,
>> +	},
>> +	.probe	= hisi_thermal_probe,
>> +	.remove	= hisi_thermal_remove,
>> +};
>> +
>> +module_platform_driver(hisi_thermal_driver);
>> +
>> +MODULE_AUTHOR("Xinwei Kong <kong.kongxinwei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>");
>> +MODULE_AUTHOR("Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
>> +MODULE_DESCRIPTION("Hisilicon thermal driver");
>> +MODULE_LICENSE("GPL v2");
> 
> 
> Otherwise the driver looks good to me.
> 
> Please fix the above and I believe the driver is ready to be merged. I
> will queue for next merge window.

:-) I believe you. :-)

> 
>> -- 
>> 1.9.1
>>
>>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/3] thermal: hisilicon: add new hisilicon thermal sensor driver

[Eduardo Valentin]

[-- Attachment #1: Type: text/plain, Size: 14877 bytes --]

On Mon, May 18, 2015 at 03:11:48PM +0800, Xinwei Kong wrote:
> From: kongxinwei <kong.kongxinwei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> 
> This patch adds the support for hisilicon thermal sensor, within
> hisilicon SoC. there will register sensors for thermal framework
> and use device tree to bind cooling device.
> 
> Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: kongxinwei <kong.kongxinwei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---
>  drivers/thermal/Kconfig        |   8 +
>  drivers/thermal/Makefile       |   1 +
>  drivers/thermal/hisi_thermal.c | 418 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 427 insertions(+)
>  create mode 100644 drivers/thermal/hisi_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index af40db0..81aee01 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -136,6 +136,14 @@ config THERMAL_EMULATION
>  	  because userland can easily disable the thermal policy by simply
>  	  flooding this sysfs node with low temperature values.
>  
> +config HISI_THERMAL
> +	tristate "Hisilicon thermal driver"
> +	depends on ARCH_HISI && CPU_THERMAL && OF
> +	help
> +	  Enable this to plug hisilicon's thermal sensor driver into the Linux
> +	  thermal framework. cpufreq is used as the cooling device to throttle
> +	  CPUs when the passive trip is crossed.
> +
>  config IMX_THERMAL
>  	tristate "Temperature sensor driver for Freescale i.MX SoCs"
>  	depends on CPU_THERMAL
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index fa0dc48..08ae7ac 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -39,3 +39,4 @@ obj-$(CONFIG_TI_SOC_THERMAL)	+= ti-soc-thermal/
>  obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
>  obj-$(CONFIG_ST_THERMAL)	+= st/
>  obj-$(CONFIG_TEGRA_SOCTHERM)	+= tegra_soctherm.o
> +obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
> new file mode 100644
> index 0000000..e0a466d
> --- /dev/null
> +++ b/drivers/thermal/hisi_thermal.c
> @@ -0,0 +1,418 @@
> +/*
> + * Hisilicon thermal sensor driver
> + *
> + * Copyright (c) 2014-2015 Hisilicon Limited.
> + * Copyright (c) 2014-2015 Linaro Limited.
> + *
> + * Xinwei Kong <kong.kongxinwei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> + * Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/cpufreq.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +


I guess you trimmed too much :-)

This code does not compile:
drivers/thermal/hisi_thermal.c: In function
‘hisi_thermal_get_sensor_temp’:
drivers/thermal/hisi_thermal.c:86:2: error: implicit declaration of
function ‘writel’ [-Werror=implicit-function-declaration]
  writel(0x0, data->regs + TEMP0_INT_EN);
    ^
drivers/thermal/hisi_thermal.c:100:2: error: implicit declaration of
function ‘readl’ [-Werror=implicit-function-declaration]
      val = readl(data->regs + TEMP0_VALUE);
        ^
cc1: some warnings being treated as errors


Please add:
#include <linux/io.h>


> +#include "thermal_core.h"
> +
> +#define TEMP0_TH			(0x4)
> +#define TEMP0_RST_TH			(0x8)
> +#define TEMP0_CFG			(0xC)
> +#define TEMP0_EN			(0x10)
> +#define TEMP0_INT_EN			(0x14)
> +#define TEMP0_INT_CLR			(0x18)
> +#define TEMP0_RST_MSK			(0x1C)
> +#define TEMP0_VALUE			(0x28)
> +
> +#define HISI_TEMP_BASE			(-60)
> +#define HISI_TEMP_RESET			(100000)
> +
> +#define HISI_MAX_SENSORS		4
> +
> +struct hisi_thermal_sensor {
> +	struct hisi_thermal_data *thermal;
> +	struct thermal_zone_device *tzd;
> +
> +	long sensor_temp;
> +	uint32_t id;
> +	uint32_t thres_temp;
> +};
> +
> +struct hisi_thermal_data {
> +	struct mutex thermal_lock;    /* protects register data */
> +	struct platform_device *pdev;
> +	struct clk *clk;
> +	struct hisi_thermal_sensor sensors[HISI_MAX_SENSORS];
> +
> +	int irq, irq_bind_sensor;
> +	bool irq_enabled;
> +
> +	void __iomem *regs;
> +};
> +
> +/* in millicelsius */
> +static inline int _step_to_temp(int step)
> +{
> +	/*
> +	 * Every step equals (1 * 200) / 255 celsius, and finally
> +	 * need convert to millicelsius.
> +	 */
> +	return (HISI_TEMP_BASE + (step * 200 / 255)) * 1000;
> +}
> +
> +static inline long _temp_to_step(long temp)
> +{
> +	return ((temp / 1000 - HISI_TEMP_BASE) * 255 / 200);
> +}
> +
> +static long hisi_thermal_get_sensor_temp(struct hisi_thermal_data *data,
> +					 struct hisi_thermal_sensor *sensor)
> +{
> +	long val;
> +
> +	mutex_lock(&data->thermal_lock);
> +
> +	/* disable interrupt */
> +	writel(0x0, data->regs + TEMP0_INT_EN);
> +	writel(0x1, data->regs + TEMP0_INT_CLR);
> +
> +	/* disable module firstly */
> +	writel(0x0, data->regs + TEMP0_EN);
> +
> +	/* select sensor id */
> +	writel((sensor->id << 12), data->regs + TEMP0_CFG);
> +
> +	/* enable module */
> +	writel(0x1, data->regs + TEMP0_EN);
> +
> +	usleep_range(3000, 5000);
> +
> +	val = readl(data->regs + TEMP0_VALUE);
> +	val = _step_to_temp(val);
> +
> +	mutex_unlock(&data->thermal_lock);
> +
> +	return val;
> +}
> +
> +static void hisi_thermal_enable_bind_irq_sensor
> +			(struct hisi_thermal_data *data)
> +{
> +	struct hisi_thermal_sensor *sensor;
> +
> +	sensor = &data->sensors[data->irq_bind_sensor];

This reader of irq_bind_sensor needs to be locked. So, move this line
below the mutex_lock below:

> +
> +	mutex_lock(&data->thermal_lock);
> +

<here>

> +	/* setting the hdak time */
> +	writel(0x0, data->regs + TEMP0_CFG);
> +
> +	/* disable module firstly */
> +	writel(0x0, data->regs + TEMP0_RST_MSK);
> +	writel(0x0, data->regs + TEMP0_EN);
> +
> +	/* select sensor id */
> +	writel((sensor->id << 12), data->regs + TEMP0_CFG);
> +
> +	/* enable for interrupt */
> +	writel(_temp_to_step(sensor->thres_temp) | 0x0FFFFFF00,
> +	       data->regs + TEMP0_TH);
> +
> +	writel(_temp_to_step(HISI_TEMP_RESET), data->regs + TEMP0_RST_TH);
> +
> +	/* enable module */
> +	writel(0x1, data->regs + TEMP0_RST_MSK);
> +	writel(0x1, data->regs + TEMP0_EN);
> +
> +	writel(0x0, data->regs + TEMP0_INT_CLR);
> +	writel(0x1, data->regs + TEMP0_INT_EN);
> +
> +	usleep_range(3000, 5000);
> +
> +	mutex_unlock(&data->thermal_lock);
> +}
> +
> +static void hisi_thermal_disable_sensor(struct hisi_thermal_data *data)
> +{
> +	mutex_lock(&data->thermal_lock);
> +
> +	/* disable sensor module */
> +	writel(0x0, data->regs + TEMP0_INT_EN);
> +	writel(0x0, data->regs + TEMP0_RST_MSK);
> +	writel(0x0, data->regs + TEMP0_EN);
> +
> +	mutex_unlock(&data->thermal_lock);
> +}
> +
> +static int hisi_thermal_get_temp(void *_sensor, long *temp)
> +{
> +	struct hisi_thermal_sensor *sensor = _sensor;
> +	struct hisi_thermal_data *data = sensor->thermal;
> +
> +	int sensor_id = 0, i;
> +	long max_temp = 0;
> +
> +	*temp = hisi_thermal_get_sensor_temp(data, sensor);
> +
> +	sensor->sensor_temp = *temp;
> +
> +	for (i = 0; i < HISI_MAX_SENSORS; i++) {
> +		if (data->sensors[i].sensor_temp >= max_temp) {
> +			max_temp = data->sensors[i].sensor_temp;
> +			sensor_id = i;
> +		}
> +	}
> +
> +	mutex_lock(&data->thermal_lock);
> +	data->irq_bind_sensor = sensor_id;
> +	mutex_unlock(&data->thermal_lock);
> +
> +	dev_dbg(&data->pdev->dev, "id=%d, irq=%d, temp=%ld, thres=%d\n",
> +		sensor->id, data->irq_enabled, *temp, sensor->thres_temp);
> +	/*
> +	 * Bind irq to sensor for two cases:
> +	 *   Reenable alarm IRQ if temperature below threshold;
> +	 *   if irq has been enabled, always set it;
> +	 */
> +	if (data->irq_enabled) {
> +		hisi_thermal_enable_bind_irq_sensor(data);
> +		return 0;
> +	}
> +
> +	if (max_temp < sensor->thres_temp) {
> +		data->irq_enabled = true;
> +		hisi_thermal_enable_bind_irq_sensor(data);
> +		enable_irq(data->irq);
> +	}
> +
> +	return 0;
> +}
> +
> +static struct thermal_zone_of_device_ops hisi_of_thermal_ops = {
> +	.get_temp = hisi_thermal_get_temp,
> +};
> +
> +static irqreturn_t hisi_thermal_alarm_irq(int irq, void *dev)
> +{
> +	struct hisi_thermal_data *data = dev;
> +
> +	disable_irq_nosync(irq);
> +	data->irq_enabled = false;
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
> +{
> +	struct hisi_thermal_data *data = dev;
> +	struct hisi_thermal_sensor *sensor;
> +	int i;
> +
> +	sensor = &data->sensors[data->irq_bind_sensor];
> +

this reader needs to be locked too.

> +	dev_crit(&data->pdev->dev, "THERMAL ALARM: T > %d\n",
> +		 sensor->thres_temp / 1000);
> +
> +	for (i = 0; i < HISI_MAX_SENSORS; i++)
> +		thermal_zone_device_update(data->sensors[i].tzd);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int hisi_thermal_register_sensor(struct platform_device *pdev,
> +					struct hisi_thermal_data *data,
> +					struct hisi_thermal_sensor *sensor,
> +					int index)
> +{
> +	int ret, i;
> +	const struct thermal_trip *trip;
> +
> +	sensor->id = index;
> +	sensor->thermal = data;
> +
> +	sensor->tzd = thermal_zone_of_sensor_register(&pdev->dev, sensor->id,
> +				sensor, &hisi_of_thermal_ops);
> +	if (IS_ERR(sensor->tzd)) {
> +		ret = PTR_ERR(sensor->tzd);
> +		dev_err(&pdev->dev, "failed to register sensor id %d: %d\n",
> +			sensor->id, ret);
> +		return ret;
> +	}
> +
> +	trip = of_thermal_get_trip_points(sensor->tzd);
> +
> +	for (i = 0; i < of_thermal_get_ntrips(sensor->tzd); i++) {
> +		if (trip[i].type == THERMAL_TRIP_PASSIVE) {
> +			sensor->thres_temp = trip[i].temperature;
> +			break;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id of_hisi_thermal_match[] = {
> +	{ .compatible = "hisilicon,tsensor" },
> +	{ /* end */ }
> +};
> +MODULE_DEVICE_TABLE(of, of_hisi_thermal_match);
> +
> +static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
> +				       bool on)
> +{
> +	struct thermal_zone_device *tzd = sensor->tzd;
> +
> +	tzd->ops->set_mode(tzd,
> +		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
> +}
> +
> +static int hisi_thermal_probe(struct platform_device *pdev)
> +{
> +	struct hisi_thermal_data *data;
> +	struct resource *res;
> +	int i;
> +	int ret;
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	mutex_init(&data->thermal_lock);
> +	data->pdev = pdev;
> +
> +	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, "failed to get io address\n");
> +		return PTR_ERR(data->regs);
> +	}
> +
> +	data->irq = platform_get_irq(pdev, 0);
> +	if (data->irq < 0)
> +		return data->irq;
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, data->irq,
> +					hisi_thermal_alarm_irq,
> +					hisi_thermal_alarm_irq_thread,
> +					0, "hisi_thermal", data);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to request alarm irq: %d\n", ret);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, data);
> +
> +	data->clk = devm_clk_get(&pdev->dev, "thermal_clk");
> +	if (IS_ERR(data->clk)) {
> +		ret = PTR_ERR(data->clk);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&pdev->dev,
> +				"failed to get thermal clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* enable clock for thermal */
> +	ret = clk_prepare_enable(data->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	for (i = 0; i < HISI_MAX_SENSORS; ++i) {
> +		ret = hisi_thermal_register_sensor(pdev, data,
> +						   &data->sensors[i], i);
> +		if (ret) {
> +			dev_err(&pdev->dev,
> +				"failed to register thermal sensor: %d\n", ret);
> +			goto err_get_sensor_data;
> +		}
> +	}
> +
> +	hisi_thermal_enable_bind_irq_sensor(data);
> +	data->irq_enabled = true;
> +
> +	for (i = 0; i < HISI_MAX_SENSORS; i++)
> +		hisi_thermal_toggle_sensor(&data->sensors[i], true);
> +
> +	return 0;
> +
> +err_get_sensor_data:
> +	clk_disable_unprepare(data->clk);
> +
> +	return ret;
> +}
> +
> +static int hisi_thermal_remove(struct platform_device *pdev)
> +{
> +	struct hisi_thermal_data *data = platform_get_drvdata(pdev);
> +	int i;
> +
> +	for (i = 0; i < HISI_MAX_SENSORS; i++) {
> +		struct hisi_thermal_sensor *sensor = &data->sensors[i];
> +
> +		hisi_thermal_toggle_sensor(sensor, false);
> +		thermal_zone_of_sensor_unregister(&pdev->dev, sensor->tzd);
> +	}
> +
> +	hisi_thermal_disable_sensor(data);
> +	clk_disable_unprepare(data->clk);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int hisi_thermal_suspend(struct device *dev)
> +{
> +	struct hisi_thermal_data *data = dev_get_drvdata(dev);
> +
> +	hisi_thermal_disable_sensor(data);
> +	data->irq_enabled = false;
> +
> +	clk_disable_unprepare(data->clk);
> +
> +	return 0;
> +}
> +
> +static int hisi_thermal_resume(struct device *dev)
> +{
> +	struct hisi_thermal_data *data = dev_get_drvdata(dev);
> +
> +	clk_prepare_enable(data->clk);
> +
> +	data->irq_enabled = true;
> +	hisi_thermal_enable_bind_irq_sensor(data);
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(hisi_thermal_pm_ops,
> +			 hisi_thermal_suspend, hisi_thermal_resume);
> +
> +static struct platform_driver hisi_thermal_driver = {
> +	.driver = {
> +		.name		= "hisi_thermal",
> +		.owner		= THIS_MODULE,
> +		.pm		= &hisi_thermal_pm_ops,
> +		.of_match_table = of_hisi_thermal_match,
> +	},
> +	.probe	= hisi_thermal_probe,
> +	.remove	= hisi_thermal_remove,
> +};
> +
> +module_platform_driver(hisi_thermal_driver);
> +
> +MODULE_AUTHOR("Xinwei Kong <kong.kongxinwei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>");
> +MODULE_AUTHOR("Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
> +MODULE_DESCRIPTION("Hisilicon thermal driver");
> +MODULE_LICENSE("GPL v2");


Otherwise the driver looks good to me.

Please fix the above and I believe the driver is ready to be merged. I
will queue for next merge window.

> -- 
> 1.9.1
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH v5 3/3] dts: hi6220: enable thermal sensor for hisilicon SoC

[Eduardo Valentin]

[-- Attachment #1: Type: text/plain, Size: 4245 bytes --]

On Mon, May 18, 2015 at 03:11:49PM +0800, Xinwei Kong wrote:
> From: kongxinwei <kong.kongxinwei@hisilicon.com>
> 
> Dts includes two part: the first part is related with thermal sensor;
> the second part is related with thermal zones, in this part it will
> define the thermal zones and which sensor device should be bound to.
> it also need specify the polling interval for every thermal zone.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: kongxinwei <kong.kongxinwei@hisilicon.com>
> ---
>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 127 ++++++++++++++++++++++++++++++
>  1 file changed, 127 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> 
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> new file mode 100644
> index 0000000..82f213d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -0,0 +1,127 @@
> +
> +#include <dt-bindings/thermal/thermal.h>
> +
> +/ {
> +
> +	tsensor: tsensor@0,f7030700 {
> +		compatible = "hisilicon,tsensor";
> +		reg = <0x0 0xf7030700 0x0 0x1000>;
> +		interrupts = <0 7 0x4>;
> +		clocks = <&clock_sys HI6220_TSENSOR_CLK>;
> +		clock-names = "thermal_clk";
> +		#thermal-sensor-cells = <1>;
> +
> +	thermal-zones {

The thermal-zones node is typically in the root node, not inside the
sensor node.

> +		local: local {
> +			/* milliseconds */
> +			polling-delay-passive = <1000>;
> +			/* milliseconds */
> +			polling-delay = <5000>;
> +
> +			/* sensor	ID */
> +			thermal-sensors = <&tsensor  0>;
> +
> +			trips {
> +				local_alert: local_alert {
> +					/* millicelsius */
> +					temperature = <70000>;
> +					/* millicelsius */
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +				local_crit: local_crit {
> +					temperature = <90000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
> +
> +			cooling-maps {
> +				/* There are currently no cooling maps
> +				because there are no cooling devices */

Not even cpufreq? in your driver you mention about cpufreq cooling.

Do you have cpufreq-dt driver  properly setup in your board?

> +			};
> +		};
> +
> +		cluster1: cluster1 {
> +			polling-delay-passive = <1000>;
> +			polling-delay = <5000>;
> +
> +			/* sensor	ID */
> +			thermal-sensors = <&tsensor  1>;
> +
> +			trips {
> +				cluster1_alert: cluster1_alert {
> +					temperature = <70000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +				cluster1_crit: cluster1_crit {
> +					temperature = <90000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
> +
> +			cooling-maps {
> +				/* There are currently no cooling maps
> +				because there are no cooling devices */
> +			};
> +		};
> +
> +		cluster0: cluster0 {
> +			polling-delay-passive = <1000>;
> +			polling-delay = <5000>;
> +
> +			/* sensor	ID */
> +			thermal-sensors = <&tsensor  2>;
> +
> +			trips {
> +				cluster0_alert: cluster0_alert {
> +					temperature = <70000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +				cluster0_crit: cluster0_crit {
> +					temperature = <90000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
> +
> +			cooling-maps {
> +				map0 {
> +					trip = <&cluster0_alert>;
> +					cooling-device =
> +					    <&cpu0 THERMAL_NO_LIMIT
> +						THERMAL_NO_LIMIT>;
> +				};
> +			};
> +		};
> +
> +		gpu: gpu {
> +			polling-delay-passive = <1000>;
> +			polling-delay = <5000>;
> +
> +			/* sensor	ID */
> +			thermal-sensors = <&tsensor  3>;
> +
> +			trips {
> +				gpu_alert: gpu_alert {
> +					temperature = <70000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +				gpu_crit: gpu_crit {
> +					temperature = <90000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
> +
> +			cooling-maps {
> +				/* There are currently no cooling maps
> +				because there are no cooling devices */
> +			};
> +		};
> +	};
> +}

Are you sure this DT file works? I think it misses a closing \}.

> -- 
> 1.9.1
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]