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

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

[kongxinwei]

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.

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

 .../bindings/thermal/hisilicon-thermal.txt         |  51 ++
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi          | 153 ++++++
 drivers/thermal/Kconfig                            |   8 +
 drivers/thermal/Makefile                           |   1 +
 drivers/thermal/hisi_thermal.c                     | 531 +++++++++++++++++++++
 5 files changed, 744 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 1/3] thermal: hisilicon: add new hisilicon thermal sensor driver

[kongxinwei]

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 | 531 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 540 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..438b1d1
--- /dev/null
+++ b/drivers/thermal/hisi_thermal.c
@@ -0,0 +1,531 @@
+/*
+ * 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/cpu_cooling.h>
+#include <linux/cpufreq.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/thermal.h>
+#include <linux/types.h>
+
+#define TEMP0_LAG			(0x0)
+#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_RAW_INT			(0x20)
+#define TEMP0_MSK_INT			(0x24)
+#define TEMP0_VALUE			(0x28)
+
+#define HISI_TEMP_BASE			(-60)
+#define HISI_TEMP_PASSIVE		(85000)
+
+#define HISI_MAX_SENSORS		4
+
+struct hisi_thermal_sensor {
+	struct hisi_thermal_data *thermal;
+	struct thermal_zone_device *tzd;
+
+	uint32_t id;
+	uint32_t lag;
+	uint32_t thres_temp, reset_temp;
+};
+
+struct hisi_thermal_data {
+	struct platform_device *pdev;
+	struct clk *clk;
+
+	int irq, irq_bind_sensor;
+	bool irq_enabled;
+
+	unsigned int sensors_num;
+	struct hisi_thermal_sensor sensors[HISI_MAX_SENSORS];
+
+	void __iomem *regs;
+};
+
+static DEFINE_SPINLOCK(thermal_lock);
+
+/* 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 int _temp_to_step(int 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)
+{
+	unsigned long flags;
+	int val;
+
+	spin_lock_irqsave(&thermal_lock, flags);
+
+	/* 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);
+
+	mdelay(5);
+
+	val = readl(data->regs + TEMP0_VALUE);
+	val = _step_to_temp(val);
+
+	/* adjust for negative value */
+	val = (val < 0) ? 0 : val;
+
+	spin_unlock_irqrestore(&thermal_lock, flags);
+
+	return val;
+}
+
+static void hisi_thermal_bind_irq(struct hisi_thermal_data *data)
+{
+	struct hisi_thermal_sensor *sensor;
+	unsigned long flags;
+
+	sensor = &data->sensors[data->irq_bind_sensor];
+
+	spin_lock_irqsave(&thermal_lock, flags);
+
+	/* 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(sensor->reset_temp), 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);
+
+	mdelay(5);
+
+	spin_unlock_irqrestore(&thermal_lock, flags);
+}
+
+static void hisi_thermal_enable_sensor(struct hisi_thermal_data *data)
+{
+	struct hisi_thermal_sensor *sensor;
+	unsigned long flags;
+
+	sensor = &data->sensors[data->irq_bind_sensor];
+
+	spin_lock_irqsave(&thermal_lock, flags);
+
+	/* 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(sensor->reset_temp), 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);
+
+	mdelay(5);
+
+	spin_unlock_irqrestore(&thermal_lock, flags);
+}
+
+static void hisi_thermal_disable_sensor(struct hisi_thermal_data *data)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&thermal_lock, flags);
+
+	/* disable sensor module */
+	writel(0x0, data->regs + TEMP0_INT_EN);
+	writel(0x0, data->regs + TEMP0_RST_MSK);
+	writel(0x0, data->regs + TEMP0_EN);
+
+	spin_unlock_irqrestore(&thermal_lock, flags);
+}
+
+static int hisi_thermal_get_temp(void *_sensor, long *temp)
+{
+	struct hisi_thermal_sensor *sensor = _sensor;
+	struct hisi_thermal_data *data = sensor->thermal;
+
+	*temp = hisi_thermal_get_sensor_temp(data, sensor);
+
+	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 && *temp < sensor->thres_temp) {
+		data->irq_enabled = true;
+		hisi_thermal_bind_irq(data);
+	} else if (data->irq_enabled)
+		hisi_thermal_bind_irq(data);
+
+	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;
+	unsigned long flags;
+
+	spin_lock_irqsave(&thermal_lock, flags);
+
+	/* disable interrupt */
+	writel(0x0, data->regs + TEMP0_INT_EN);
+	writel(0x1, data->regs + TEMP0_INT_CLR);
+
+	spin_unlock_irqrestore(&thermal_lock, flags);
+	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 < data->sensors_num; i++)
+		thermal_zone_device_update(data->sensors[i].tzd);
+
+	return IRQ_HANDLED;
+}
+
+static int hisi_thermal_init_sensor(struct platform_device *pdev,
+				    struct device_node *np,
+				    struct hisi_thermal_data *data,
+				    int index)
+{
+	struct hisi_thermal_sensor *sensor;
+	int ret;
+
+	sensor = &data->sensors[index];
+
+	ret = of_property_read_u32(np, "hisilicon,tsensor-id",
+				   &sensor->id);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to get id %d: %d\n", index, ret);
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "hisilicon,tsensor-lag-value",
+				   &sensor->lag);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to get lag %d: %d\n", index, ret);
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "hisilicon,tsensor-thres-temp",
+				   &sensor->thres_temp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to get thres value %d: %d\n",
+			index, ret);
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "hisilicon,tsensor-reset-temp",
+				   &sensor->reset_temp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to get reset value %d: %d\n",
+			index, ret);
+		return ret;
+	}
+
+	if (of_property_read_bool(np, "hisilicon,tsensor-bind-irq")) {
+
+		if (data->irq_bind_sensor != -1)
+			dev_warn(&pdev->dev, "irq has bound to index %d\n",
+				 data->irq_bind_sensor);
+
+		/* bind irq to this sensor */
+		data->irq_bind_sensor = 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;
+	}
+
+	return 0;
+}
+
+static int hisi_thermal_get_sensor_data(struct platform_device *pdev)
+{
+	struct hisi_thermal_data *data = platform_get_drvdata(pdev);
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child_np;
+	int ret, i, index;
+
+	data->irq_bind_sensor = -1;
+
+	/* sensor initialization */
+	index = 0;
+	for_each_child_of_node(np, child_np) {
+
+		if (index >= HISI_MAX_SENSORS) {
+			dev_err(&pdev->dev, "unsupported sensor number\n");
+			ret = -EINVAL;
+			goto err_init_sensors;
+		}
+
+		ret = hisi_thermal_init_sensor(pdev, child_np, data, index);
+		if (ret)
+			goto err_init_sensors;
+
+		index++;
+	}
+
+	if (data->irq_bind_sensor  == -1) {
+		dev_err(&pdev->dev, "don't bind irq for sensor\n");
+		ret = -EINVAL;
+		goto err_init_sensors;
+	}
+
+	data->sensors_num = index;
+	return 0;
+
+err_init_sensors:
+	for (i = 0; i < index; i++)
+		thermal_zone_of_sensor_unregister(&pdev->dev,
+				data->sensors[i].tzd);
+	return ret;
+}
+
+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;
+
+	if (!cpufreq_get_current_driver()) {
+		dev_dbg(&pdev->dev, "no cpufreq driver!");
+		return -EPROBE_DEFER;
+	}
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	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, NULL);
+	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;
+	}
+
+	ret = hisi_thermal_get_sensor_data(pdev);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to get sensor data\n");
+		goto err_get_sensor_data;
+		return ret;
+	}
+
+	hisi_thermal_enable_sensor(data);
+	data->irq_enabled = true;
+
+	for (i = 0; i < data->sensors_num; 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 < data->sensors_num; 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_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 2/3] dts: hi6220: enable thermal sensor for hisilicon SoC

[kongxinwei]

There have two parts for thermal sensor:

1. The first part is related with thermal sensor, every sensor need to
pass the sensor id, the threshold and reset temperature; so the sensor
will be registered as sensor devices;

2. 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 | 153 ++++++++++++++++++++++++++++++
 1 file changed, 153 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..da49790
--- /dev/null
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -0,0 +1,153 @@
+
+#include <dt-bindings/thermal/thermal.h>
+
+/ {
+
+	tsensor: tsensor at 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>;
+
+		local_sensor {
+			hisilicon,tsensor-id = <0>;
+			hisilicon,tsensor-thres-temp = <80000>;
+			hisilicon,tsensor-reset-temp = <100000>;
+			hisilicon,tsensor-bind-irq;
+		};
+
+		acpu1_sensor {
+			hisilicon,tsensor-id = <1>;
+			hisilicon,tsensor-thres-temp = <80000>;
+			hisilicon,tsensor-reset-temp = <100000>;
+		};
+
+		acpu0_sensor {
+			hisilicon,tsensor-id = <2>;
+			hisilicon,tsensor-thres-temp = <80000>;
+			hisilicon,tsensor-reset-temp = <100000>;
+		};
+
+		gpu_sensor {
+			hisilicon,tsensor-id = <3>;
+			hisilicon,tsensor-thres-temp = <80000>;
+			hisilicon,tsensor-reset-temp = <100000>;
+		};
+	};
+
+	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

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

[kongxinwei]

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         | 51 ++++++++++++++++++++++
 1 file changed, 51 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..b75c48e
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt
@@ -0,0 +1,51 @@
+* Hisilicon Thermal
+
+This driver is for hi6220 SoC which contain 4 thermal sensor.
+
+	1. sensor 0: local sensor;
+	2. sensor 1: remote sensor for ACPU cluster 1;
+	3. sensor 2: remote sensor for ACPU cluster 2;
+	4. sensor 3: remote sensor for GPU.
+
+Every sensor use one child node to represent it, so thermal sensor include
+parent node and four child node. The parent node describe common feature and
+child node describe private feature for thermal sensor;
+
+** Required properties :
+
+- compatible : "hisilicon,tsensor";
+- reg : address range of the thermal sensor registers;
+- interrupt : Standard way to define interrupt numbr;
+- clock-names : Should be "thermal_clk".
+		  See: Documentation/devicetree/bindings/resource-names.txt;
+- clocks : Phandle of the clock used by the thermal sensor.
+	     See: Documentation/devicetree/bindings/clock/clock-bindingm.txt
+- #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description;
+
+** Required properties for child nodes :
+
+- hisilicon,tsensor-id : the index of thermal senor;
+- hisilicon,tsensor-thres-temp : the interrupt threshold temperature of
+				thermal senor;
+- hisilicon,tsensor-reset-temp : the reset temperature of the hardware SoC;
+- hisilicon,tsensor-bind-irq : systerm interrupt binding thermal sensor;
+
+Example :
+
+	tsensor: tsensor at 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>;
+
+		local_sensor {
+			hisilicon,tsensor-id = <0>;
+			hisilicon,tsensor-lag-value = <10>;
+			hisilicon,tsensor-thres-temp = <35000>;
+			hisilicon,tsensor-reset-temp = <100000>;
+			hisilicon,tsensor-bind-irq;
+		}
+		.......
+	}
-- 
1.9.1

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

[Mark Rutland]

On Thu, Mar 19, 2015 at 07:57:26AM +0000, kongxinwei wrote:
> 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.
> 
> kongxinwei (3):
>   thermal: hisilicon: add new hisilicon thermal sensor driver
>   dts: hi6220: enable thermal sensor for hisilicon SoC
>   dt-bindings: Document the hi6220 thermal sensor bindings
> 
>  .../bindings/thermal/hisilicon-thermal.txt         |  51 ++
>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi          | 153 ++++++
>  drivers/thermal/Kconfig                            |   8 +
>  drivers/thermal/Makefile                           |   1 +
>  drivers/thermal/hisi_thermal.c                     | 531 +++++++++++++++++++++

Mainline does not have a hi6220 dtsi.

Which tree is this against?

What are your dependencies?

Mark.

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

[Mark Rutland]

On Thu, Mar 19, 2015 at 07:57:29AM +0000, kongxinwei wrote:
> This adds documentation of device tree bindings for the
> thermal sensor controller of hi6220 SoC.

Please place the patches adding binding docs _before_ any patcheds
introducing code or dts files using them.

> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: kongxinwei <kong.kongxinwei@hisilicon.com>
> ---
>  .../bindings/thermal/hisilicon-thermal.txt         | 51 ++++++++++++++++++++++
>  1 file changed, 51 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..b75c48e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt
> @@ -0,0 +1,51 @@
> +* Hisilicon Thermal
> +
> +This driver is for hi6220 SoC which contain 4 thermal sensor.
> +
> +	1. sensor 0: local sensor;
> +	2. sensor 1: remote sensor for ACPU cluster 1;
> +	3. sensor 2: remote sensor for ACPU cluster 2;
> +	4. sensor 3: remote sensor for GPU.
> +
> +Every sensor use one child node to represent it, so thermal sensor include
> +parent node and four child node. The parent node describe common feature and
> +child node describe private feature for thermal sensor;
> +
> +** Required properties :
> +
> +- compatible : "hisilicon,tsensor";
> +- reg : address range of the thermal sensor registers;

Is there jsut the one block?

> +- interrupt : Standard way to define interrupt numbr;

Describe what the interrupt logically is from the PoV of this device. We
all know what an interrupt is in abstract.

> +- clock-names : Should be "thermal_clk".

What is the clock input actually called on the data sheet?

Is this the only clock input?

No regulators or other inputs we need to describe?

> +		  See: Documentation/devicetree/bindings/resource-names.txt;

Drop this, it's irrelevant.

> +- clocks : Phandle of the clock used by the thermal sensor.

Define this in terms of clock-names.

> +	     See: Documentation/devicetree/bindings/clock/clock-bindingm.txt

Drop this. This is pointless, exepcially given the typo.

> +- #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description;

Define what the permitted values are. This is useless as-is.

> +
> +** Required properties for child nodes :
> +
> +- hisilicon,tsensor-id : the index of thermal senor;

As with #thermal-sensor-cells, this description is incomplete.

> +- hisilicon,tsensor-thres-temp : the interrupt threshold temperature of
> +				thermal senor;
> +- hisilicon,tsensor-reset-temp : the reset temperature of the hardware SoC;

NAK. These do not belong on the sensor.

> +- hisilicon,tsensor-bind-irq : systerm interrupt binding thermal sensor;

NAK. I've tried a few times, but I still can't figure out what this is
intended to mean. I don't see what this could possibly mean that would
be reasonable to describe.

Mark.

> +
> +Example :
> +
> +	tsensor: tsensor at 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>;
> +
> +		local_sensor {
> +			hisilicon,tsensor-id = <0>;
> +			hisilicon,tsensor-lag-value = <10>;
> +			hisilicon,tsensor-thres-temp = <35000>;
> +			hisilicon,tsensor-reset-temp = <100000>;
> +			hisilicon,tsensor-bind-irq;
> +		}
> +		.......
> +	}
> -- 
> 1.9.1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Mark Rutland]

On Thu, Mar 19, 2015 at 07:57:27AM +0000, kongxinwei wrote:
> 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 | 531 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 540 insertions(+)
>  create mode 100644 drivers/thermal/hisi_thermal.c

[...]

> +       ret = of_property_read_u32(np, "hisilicon,tsensor-lag-value",
> +                                  &sensor->lag);

This wasn't in the binding.

[...]

> +       ret = of_property_read_u32(np, "hisilicon,tsensor-thres-temp",
> +                                  &sensor->thres_temp);
> +       if (ret) {
> +               dev_err(&pdev->dev, "failed to get thres value %d: %d\n",
> +                       index, ret);
> +               return ret;
> +       }
> +
> +       ret = of_property_read_u32(np, "hisilicon,tsensor-reset-temp",
> +                                  &sensor->reset_temp);
> +       if (ret) {
> +               dev_err(&pdev->dev, "failed to get reset value %d: %d\n",
> +                       index, ret);
> +               return ret;
> +       }

I see now that these properties result in the HW being programmed. You
should figure out how to reconcile these with thermal-zone trip points
rather than having parallel properties.

> +
> +       if (of_property_read_bool(np, "hisilicon,tsensor-bind-irq")) {
> +
> +               if (data->irq_bind_sensor != -1)
> +                       dev_warn(&pdev->dev, "irq has bound to index %d\n",
> +                                data->irq_bind_sensor);
> +
> +               /* bind irq to this sensor */
> +               data->irq_bind_sensor = index;
> +       }

I don't see why this should be specified in the DT. Why do you believe
it should?

[...]

> +static int hisi_thermal_probe(struct platform_device *pdev)
> +{
> +       struct hisi_thermal_data *data;
> +       struct resource *res;
> +       int i;
> +       int ret;
> +
> +       if (!cpufreq_get_current_driver()) {
> +               dev_dbg(&pdev->dev, "no cpufreq driver!");
> +               return -EPROBE_DEFER;
> +       }

Surely we care about not burning out the board even if we don't have
cpufreq?

Is there any ordering guarantee between the probing of this driver and
cpufreq?


[...]

> +       data->clk = devm_clk_get(&pdev->dev, NULL);

You gave this clock a name in the binding. Use it or drop it.

Mark. 

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

[kongxinwei]

hi Mark Rutland:

? 2015/3/19 21:59, Mark Rutland ??:
> On Thu, Mar 19, 2015 at 07:57:26AM +0000, kongxinwei wrote:
>> 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.
>>
>> kongxinwei (3):
>>   thermal: hisilicon: add new hisilicon thermal sensor driver
>>   dts: hi6220: enable thermal sensor for hisilicon SoC
>>   dt-bindings: Document the hi6220 thermal sensor bindings
>>
>>  .../bindings/thermal/hisilicon-thermal.txt         |  51 ++
>>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi          | 153 ++++++
>>  drivers/thermal/Kconfig                            |   8 +
>>  drivers/thermal/Makefile                           |   1 +
>>  drivers/thermal/hisi_thermal.c                     | 531 +++++++++++++++++++++
> 
> Mainline does not have a hi6220 dtsi.
> 
> Which tree is this against?
> 
> What are your dependencies?
> 
> Mark.
> 
Hikey board code are being upstreamed by our team.If hi6220 dtsi be accepted, this patch is ok.

Xinwei

> .
> 

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

[Leo Yan]

hi Mark,

Thanks for reviewing, pls see below questions.

On Thu, Mar 19, 2015 at 02:17:53PM +0000, Mark Rutland wrote:
> On Thu, Mar 19, 2015 at 07:57:27AM +0000, kongxinwei wrote:
> > 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 | 531 +++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 540 insertions(+)
> >  create mode 100644 drivers/thermal/hisi_thermal.c

[...]

> > +       ret = of_property_read_u32(np, "hisilicon,tsensor-thres-temp",
> > +                                  &sensor->thres_temp);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "failed to get thres value %d: %d\n",
> > +                       index, ret);
> > +               return ret;
> > +       }
> > +
> > +       ret = of_property_read_u32(np, "hisilicon,tsensor-reset-temp",
> > +                                  &sensor->reset_temp);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "failed to get reset value %d: %d\n",
> > +                       index, ret);
> > +               return ret;
> > +       }
> 
> I see now that these properties result in the HW being programmed. You
> should figure out how to reconcile these with thermal-zone trip points
> rather than having parallel properties.
 
Set "tsensor-thres-temp" to register so that if thermal reaches the
threshold, the sensor will trigger h/w interrupt.

Set "tsensor-reset-temp" to register so that if thermal reaches the
reset value, the sensor will assert SoC reset signal to trigger h/w
reset.

This is different w/t thermal-zone trip points, the trip points are
used for timer polling. Do u think below modification is more
reasonable?

- Set "tsensor-thres-temp" = 700000, which equal to thermal-zone
  passive trip point, so that we can use h/w interrupt to update the
  thermal value immediately, rather than using polling method w/t long
  delay;

- Set "tsensor-reset-temp" = <100000>, which is higher than
  thermal-zone critical trip point, so that the s/w reset method has
  higher priority than h/w reset; we also can easily know the reset is
  caused by thermal framework; "tsensor-reset-temp" is only used to
  protect h/w circuit.

	tsensor: tsensor at 0,f7030700 {
		acpu1_sensor {
			hisilicon,tsensor-id = <1>;
			hisilicon,tsensor-lag-value = <10>;
			hisilicon,tsensor-thres-temp = <70000>;
			hisilicon,tsensor-reset-temp = <100000>;
		};
	};

	thermal-zones {

                ...

		cluster1: cluster1 {
			polling-delay-passive = <1000>; /* milliseconds */
			polling-delay = <5000>; /* milliseconds */

			/* sensor	ID */
			thermal-sensors = <&tsensor  1>;

			trips {
				cluster1_alert: cluster1_alert {
					temperature = <70000>; /* millicelsius */
					hysteresis = <2000>; /* millicelsius */
					type = "passive";
				};
				cluster1_crit: cluster1_crit {
					temperature = <90000>; /* millicelsius */
					hysteresis = <2000>; /* millicelsius */
					type = "critical";
				};
			};
        };

> > +
> > +       if (of_property_read_bool(np, "hisilicon,tsensor-bind-irq")) {
> > +
> > +               if (data->irq_bind_sensor != -1)
> > +                       dev_warn(&pdev->dev, "irq has bound to index %d\n",
> > +                                data->irq_bind_sensor);
> > +
> > +               /* bind irq to this sensor */
> > +               data->irq_bind_sensor = index;
> > +       }
> 
> I don't see why this should be specified in the DT. Why do you believe
> it should?

The thermal sensor module has four sensors, but have only one
interrupt signal; This interrupt can only be used by one sensor;
So want to use dts to bind the interrupt with one selected sensor.

> [...]
> 
> > +static int hisi_thermal_probe(struct platform_device *pdev)
> > +{
> > +       struct hisi_thermal_data *data;
> > +       struct resource *res;
> > +       int i;
> > +       int ret;
> > +
> > +       if (!cpufreq_get_current_driver()) {
> > +               dev_dbg(&pdev->dev, "no cpufreq driver!");
> > +               return -EPROBE_DEFER;
> > +       }
> 
> Surely we care about not burning out the board even if we don't have
> cpufreq?
> 
> Is there any ordering guarantee between the probing of this driver and
> cpufreq?

Yes, here need binding the thermal sensor w/t cpu cooling device,
and cpu cooling device is based on cpufreq driver.

Thanks,
Leo Yan

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

[Leo Yan]

On Fri, Mar 20, 2015 at 11:10:16AM +0800, kongxinwei wrote:
> hi Mark Rutland:
> 
> ? 2015/3/19 21:59, Mark Rutland ??:
> > On Thu, Mar 19, 2015 at 07:57:26AM +0000, kongxinwei wrote:
> >> 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.
> >>
> >> kongxinwei (3):
> >>   thermal: hisilicon: add new hisilicon thermal sensor driver
> >>   dts: hi6220: enable thermal sensor for hisilicon SoC
> >>   dt-bindings: Document the hi6220 thermal sensor bindings
> >>
> >>  .../bindings/thermal/hisilicon-thermal.txt         |  51 ++
> >>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi          | 153 ++++++
> >>  drivers/thermal/Kconfig                            |   8 +
> >>  drivers/thermal/Makefile                           |   1 +
> >>  drivers/thermal/hisi_thermal.c                     | 531 +++++++++++++++++++++
> > 
> > Mainline does not have a hi6220 dtsi.
> > 
> > Which tree is this against?
> > 
> > What are your dependencies?
> > 
> > Mark.
> > 
> Hikey board code are being upstreamed by our team.If hi6220 dtsi be accepted, this patch is ok.

Xinwei, u could resend this patch after hi6220 dtsi related patches has
been merged. Suggest this patchset are only for thermal driver.

Thanks,
Leo Yan

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

[kongxinwei]

? 2015/3/19 22:17, Mark Rutland ??:
> On Thu, Mar 19, 2015 at 07:57:27AM +0000, kongxinwei wrote:
>> 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 | 531 +++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 540 insertions(+)
>>  create mode 100644 drivers/thermal/hisi_thermal.c
> 
> [...]
> 
>> +       ret = of_property_read_u32(np, "hisilicon,tsensor-lag-value",
>> +                                  &sensor->lag);
> 
> This wasn't in the binding.
> 

good comment, delete it.

Xinwei

> [...]
> 
>> +       ret = of_property_read_u32(np, "hisilicon,tsensor-thres-temp",
>> +                                  &sensor->thres_temp);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "failed to get thres value %d: %d\n",
>> +                       index, ret);
>> +               return ret;
>> +       }
>> +
>> +       ret = of_property_read_u32(np, "hisilicon,tsensor-reset-temp",
>> +                                  &sensor->reset_temp);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "failed to get reset value %d: %d\n",
>> +                       index, ret);
>> +               return ret;
>> +       }
> 
> I see now that these properties result in the HW being programmed. You
> should figure out how to reconcile these with thermal-zone trip points
> rather than having parallel properties.
> 
oh,firstly,this "tsensor-thres-temp" property is threshold temperature value
which causes interrupt function by setting thermal value register. "thermal
-zone trip points" applies scanning mode to cause other function such as
cooling freq .., but this "tsensor-thres-temp" property be used by interrupt
mode. when scanning mode don't satisfies systerm demands, the interrut mode
perfectly help scanning mode to complete function. "tsensor-thres-temp" temp-
erature is higher than "thermal-zone trip points" temperature, so this "
tsensor-thres-temp" property is secondary attribute.

secondly, this "tsensor-reset-temp" property is hardware protect temperature
which is close to or is below to burn out the SoC. When SoC temperature is
"tsensor-reset-temp" temperature value, SoC will be force to reboot and ensure
SoC not to burn out. So it don't conflict thermal-zone.


>> +
>> +       if (of_property_read_bool(np, "hisilicon,tsensor-bind-irq")) {
>> +
>> +               if (data->irq_bind_sensor != -1)
>> +                       dev_warn(&pdev->dev, "irq has bound to index %d\n",
>> +                                data->irq_bind_sensor);
>> +
>> +               /* bind irq to this sensor */
>> +               data->irq_bind_sensor = index;
>> +       }
> 
> I don't see why this should be specified in the DT. Why do you believe
> it should?
> 
SoC include foure thermal sensor modules,every modules is able to cause
interrupt,so binding a module to realize interupt function and i believe
it should be specified.
> [...]
> 
>> +static int hisi_thermal_probe(struct platform_device *pdev)
>> +{
>> +       struct hisi_thermal_data *data;
>> +       struct resource *res;
>> +       int i;
>> +       int ret;
>> +
>> +       if (!cpufreq_get_current_driver()) {
>> +               dev_dbg(&pdev->dev, "no cpufreq driver!");
>> +               return -EPROBE_DEFER;
>> +       }
> 
> Surely we care about not burning out the board even if we don't have
> cpufreq?
> 
> Is there any ordering guarantee between the probing of this driver and
> cpufreq?
> 
> 
Yes! It will use thermal framework to realize cpu cooling device function.

> [...]
> 
>> +       data->clk = devm_clk_get(&pdev->dev, NULL);
> 
> You gave this clock a name in the binding. Use it or drop it.
>

Thanks comment,use it.

> Mark. 
> 

Xinwei
> .
> 

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

[kongxinwei]

? 2015/3/20 14:13, Leo Yan ??:
> On Fri, Mar 20, 2015 at 11:10:16AM +0800, kongxinwei wrote:
>> hi Mark Rutland:
>>
>> ? 2015/3/19 21:59, Mark Rutland ??:
>>> On Thu, Mar 19, 2015 at 07:57:26AM +0000, kongxinwei wrote:
>>>> 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.
>>>>
>>>> kongxinwei (3):
>>>>   thermal: hisilicon: add new hisilicon thermal sensor driver
>>>>   dts: hi6220: enable thermal sensor for hisilicon SoC
>>>>   dt-bindings: Document the hi6220 thermal sensor bindings
>>>>
>>>>  .../bindings/thermal/hisilicon-thermal.txt         |  51 ++
>>>>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi          | 153 ++++++
>>>>  drivers/thermal/Kconfig                            |   8 +
>>>>  drivers/thermal/Makefile                           |   1 +
>>>>  drivers/thermal/hisi_thermal.c                     | 531 +++++++++++++++++++++
>>>
>>> Mainline does not have a hi6220 dtsi.
>>>
>>> Which tree is this against?
>>>
>>> What are your dependencies?
>>>
>>> Mark.
>>>
>> Hikey board code are being upstreamed by our team.If hi6220 dtsi be accepted, this patch is ok.
> 
> Xinwei, u could resend this patch after hi6220 dtsi related patches has
> been merged. Suggest this patchset are only for thermal driver.
> 
> Thanks,
> Leo Yan
> 

Oh,i wish that many persons review it and give some comments, then i will
send v2 patch and reduce dts file.

Thanks,
Xin Wei
> .
> 

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

[Mark Rutland]

> > > +       ret = of_property_read_u32(np, "hisilicon,tsensor-thres-temp",
> > > +                                  &sensor->thres_temp);
> > > +       if (ret) {
> > > +               dev_err(&pdev->dev, "failed to get thres value %d: %d\n",
> > > +                       index, ret);
> > > +               return ret;
> > > +       }
> > > +
> > > +       ret = of_property_read_u32(np, "hisilicon,tsensor-reset-temp",
> > > +                                  &sensor->reset_temp);
> > > +       if (ret) {
> > > +               dev_err(&pdev->dev, "failed to get reset value %d: %d\n",
> > > +                       index, ret);
> > > +               return ret;
> > > +       }
> > 
> > I see now that these properties result in the HW being programmed. You
> > should figure out how to reconcile these with thermal-zone trip points
> > rather than having parallel properties.
>  
> Set "tsensor-thres-temp" to register so that if thermal reaches the
> threshold, the sensor will trigger h/w interrupt.
> 
> Set "tsensor-reset-temp" to register so that if thermal reaches the
> reset value, the sensor will assert SoC reset signal to trigger h/w
> reset.

I understand this.

> This is different w/t thermal-zone trip points, the trip points are
> used for timer polling.

That may be the case in the code as it stands today, but per the binding
the trip points are the temperatures at which an action is to be taken.

The thermal-zone has poilling-delay and polling-delay-passive, but
there's no reason you couldn't also use the interrupt to handle the
"hot" trip-point, adn the reset at the "critical" trip-point. All that's
missing is the plumbing in order to do so.

So please co-ordinate with the thermal framework to do that.

> Do u think below modification is more reasonable?
> 
> - Set "tsensor-thres-temp" = 700000, which equal to thermal-zone
>   passive trip point, so that we can use h/w interrupt to update the
>   thermal value immediately, rather than using polling method w/t long
>   delay;
> 
> - Set "tsensor-reset-temp" = <100000>, which is higher than
>   thermal-zone critical trip point, so that the s/w reset method has
>   higher priority than h/w reset; we also can easily know the reset is
>   caused by thermal framework; "tsensor-reset-temp" is only used to
>   protect h/w circuit.

As mentioned above, I think that you should co-ordinate with the thermal
framework. You're worknig around limitations inthe code as it stands
today rather than solving the fundamental issue.

> > > +       if (of_property_read_bool(np, "hisilicon,tsensor-bind-irq")) {
> > > +
> > > +               if (data->irq_bind_sensor != -1)
> > > +                       dev_warn(&pdev->dev, "irq has bound to index %d\n",
> > > +                                data->irq_bind_sensor);
> > > +
> > > +               /* bind irq to this sensor */
> > > +               data->irq_bind_sensor = index;
> > > +       }
> > 
> > I don't see why this should be specified in the DT. Why do you believe
> > it should?
> 
> The thermal sensor module has four sensors, but have only one
> interrupt signal; This interrupt can only be used by one sensor;
> So want to use dts to bind the interrupt with one selected sensor.

That's not all that great, though I'm not exactly sure how the kernel
would select the best sensor to measure with. It would be good if you
could talk to the thermal maintainers w.r.t. this.

> > > +static int hisi_thermal_probe(struct platform_device *pdev)
> > > +{
> > > +       struct hisi_thermal_data *data;
> > > +       struct resource *res;
> > > +       int i;
> > > +       int ret;
> > > +
> > > +       if (!cpufreq_get_current_driver()) {
> > > +               dev_dbg(&pdev->dev, "no cpufreq driver!");
> > > +               return -EPROBE_DEFER;
> > > +       }
> > 
> > Surely we care about not burning out the board even if we don't have
> > cpufreq?
> > 
> > Is there any ordering guarantee between the probing of this driver and
> > cpufreq?
> 
> Yes, here need binding the thermal sensor w/t cpu cooling device,
> and cpu cooling device is based on cpufreq driver.

Sure, but if you don't have a cooling device you still want the critical
temperature reset and so on, no?

Mark.

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

[Leo Yan]

On Fri, Mar 20, 2015 at 11:24:14AM +0000, Mark Rutland wrote:
> > > > +       ret = of_property_read_u32(np, "hisilicon,tsensor-thres-temp",
> > > > +                                  &sensor->thres_temp);
> > > > +       if (ret) {
> > > > +               dev_err(&pdev->dev, "failed to get thres value %d: %d\n",
> > > > +                       index, ret);
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       ret = of_property_read_u32(np, "hisilicon,tsensor-reset-temp",
> > > > +                                  &sensor->reset_temp);
> > > > +       if (ret) {
> > > > +               dev_err(&pdev->dev, "failed to get reset value %d: %d\n",
> > > > +                       index, ret);
> > > > +               return ret;
> > > > +       }
> > > 
> > > I see now that these properties result in the HW being programmed. You
> > > should figure out how to reconcile these with thermal-zone trip points
> > > rather than having parallel properties.
> >  
> > Set "tsensor-thres-temp" to register so that if thermal reaches the
> > threshold, the sensor will trigger h/w interrupt.
> > 
> > Set "tsensor-reset-temp" to register so that if thermal reaches the
> > reset value, the sensor will assert SoC reset signal to trigger h/w
> > reset.
> 
> I understand this.
> 
> > This is different w/t thermal-zone trip points, the trip points are
> > used for timer polling.
> 
> That may be the case in the code as it stands today, but per the binding
> the trip points are the temperatures at which an action is to be taken.
> 
> The thermal-zone has poilling-delay and polling-delay-passive, but
> there's no reason you couldn't also use the interrupt to handle the
> "hot" trip-point, adn the reset at the "critical" trip-point. All that's
> missing is the plumbing in order to do so.
> 
> So please co-ordinate with the thermal framework to do that.

Let's dig further more for this point, so that we can get more specific
gudiance and have a good preparation for next version's patch set.

After i reviewed the thermal framework code, currently have one smooth
way to co-ordinate the trip points w/t thermal framework: use the function
*thermal_zone_device_register()* to register sensor, and can use the
callback function .get_trip_temp to tell thermal framework for the
trip points' temperature.

For hisi thermal case, now the driver is using the function
*thermal_zone_of_sensor_register* to register sensor, but use this way
i have not found there have standard APIs which can be used by sensor
driver to get the trip points info from thermal framework.

I may miss something for thermal framework, so if have existed APIs to
get the trip points, could pls point out?

> > Do u think below modification is more reasonable?
> > 
> > - Set "tsensor-thres-temp" = 700000, which equal to thermal-zone
> >   passive trip point, so that we can use h/w interrupt to update the
> >   thermal value immediately, rather than using polling method w/t long
> >   delay;
> > 
> > - Set "tsensor-reset-temp" = <100000>, which is higher than
> >   thermal-zone critical trip point, so that the s/w reset method has
> >   higher priority than h/w reset; we also can easily know the reset is
> >   caused by thermal framework; "tsensor-reset-temp" is only used to
> >   protect h/w circuit.
> 
> As mentioned above, I think that you should co-ordinate with the thermal
> framework. You're worknig around limitations inthe code as it stands
> today rather than solving the fundamental issue.
> 
> > > > +       if (of_property_read_bool(np, "hisilicon,tsensor-bind-irq")) {
> > > > +
> > > > +               if (data->irq_bind_sensor != -1)
> > > > +                       dev_warn(&pdev->dev, "irq has bound to index %d\n",
> > > > +                                data->irq_bind_sensor);
> > > > +
> > > > +               /* bind irq to this sensor */
> > > > +               data->irq_bind_sensor = index;
> > > > +       }
> > > 
> > > I don't see why this should be specified in the DT. Why do you believe
> > > it should?
> > 
> > The thermal sensor module has four sensors, but have only one
> > interrupt signal; This interrupt can only be used by one sensor;
> > So want to use dts to bind the interrupt with one selected sensor.
> 
> That's not all that great, though I'm not exactly sure how the kernel
> would select the best sensor to measure with. It would be good if you
> could talk to the thermal maintainers w.r.t. this.

This will be decided by the silicon, right? Every soc has different
combination with cpu/gpu/vpu, so which part is hottest, this maybe
highly dependent on individual SoC.

S/W just need provide the flexibility so that later can choose
the interrupt to bind with the sensor within the hottest part.

> > > > +static int hisi_thermal_probe(struct platform_device *pdev)
> > > > +{
> > > > +       struct hisi_thermal_data *data;
> > > > +       struct resource *res;
> > > > +       int i;
> > > > +       int ret;
> > > > +
> > > > +       if (!cpufreq_get_current_driver()) {
> > > > +               dev_dbg(&pdev->dev, "no cpufreq driver!");
> > > > +               return -EPROBE_DEFER;
> > > > +       }
> > > 
> > > Surely we care about not burning out the board even if we don't have
> > > cpufreq?
> > > 
> > > Is there any ordering guarantee between the probing of this driver and
> > > cpufreq?
> > 
> > Yes, here need binding the thermal sensor w/t cpu cooling device,
> > and cpu cooling device is based on cpufreq driver.
> 
> Sure, but if you don't have a cooling device you still want the critical
> temperature reset and so on, no?

Okay, let's remove this dependency.

Thanks,
Leo Yan

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

[Xinwei Kong]

On 2015?03?20? 19:24, Mark Rutland wrote:
>>>> +       ret = of_property_read_u32(np, "hisilicon,tsensor-thres-temp",
>>>> +                                  &sensor->thres_temp);
>>>> +       if (ret) {
>>>> +               dev_err(&pdev->dev, "failed to get thres value %d: %d\n",
>>>> +                       index, ret);
>>>> +               return ret;
>>>> +       }
>>>> +
>>>> +       ret = of_property_read_u32(np, "hisilicon,tsensor-reset-temp",
>>>> +                                  &sensor->reset_temp);
>>>> +       if (ret) {
>>>> +               dev_err(&pdev->dev, "failed to get reset value %d: %d\n",
>>>> +                       index, ret);
>>>> +               return ret;
>>>> +       }
>>> I see now that these properties result in the HW being programmed. You
>>> should figure out how to reconcile these with thermal-zone trip points
>>> rather than having parallel properties.
>>   
>> Set "tsensor-thres-temp" to register so that if thermal reaches the
>> threshold, the sensor will trigger h/w interrupt.
>>
>> Set "tsensor-reset-temp" to register so that if thermal reaches the
>> reset value, the sensor will assert SoC reset signal to trigger h/w
>> reset.
> I understand this.
>
>> This is different w/t thermal-zone trip points, the trip points are
>> used for timer polling.
> That may be the case in the code as it stands today, but per the binding
> the trip points are the temperatures at which an action is to be taken.
>
> The thermal-zone has poilling-delay and polling-delay-passive, but
> there's no reason you couldn't also use the interrupt to handle the
> "hot" trip-point, adn the reset at the "critical" trip-point. All that's
> missing is the plumbing in order to do so.
>
> So please co-ordinate with the thermal framework to do that.

In order to co-ordinate with the thermal framework, We will fix "tsensor-thres-temp"
and "tsensor-reset-temp" value in the dts. This temperatue will higher than thermal
zone trip-point value. During polling delay time if SoC temperature is above thermal
zone trip-point value and below "tsensor-thres-temp" value,the systerm will use this
thermal framework ways to realize basic function such as cpu cooling device. Otherwise
it will use interrput mode to cause some functions.

This interrupt mode help thermal framework way to complete function, in high temperature
situation this interrupt is better than other mode. I think that it can work well.

Xinwei

>> Do u think below modification is more reasonable?
>>
>> - Set "tsensor-thres-temp" = 700000, which equal to thermal-zone
>>    passive trip point, so that we can use h/w interrupt to update the
>>    thermal value immediately, rather than using polling method w/t long
>>    delay;
>>
>> - Set "tsensor-reset-temp" = <100000>, which is higher than
>>    thermal-zone critical trip point, so that the s/w reset method has
>>    higher priority than h/w reset; we also can easily know the reset is
>>    caused by thermal framework; "tsensor-reset-temp" is only used to
>>    protect h/w circuit.
> As mentioned above, I think that you should co-ordinate with the thermal
> framework. You're worknig around limitations inthe code as it stands
> today rather than solving the fundamental issue.
>
>>>> +       if (of_property_read_bool(np, "hisilicon,tsensor-bind-irq")) {
>>>> +
>>>> +               if (data->irq_bind_sensor != -1)
>>>> +                       dev_warn(&pdev->dev, "irq has bound to index %d\n",
>>>> +                                data->irq_bind_sensor);
>>>> +
>>>> +               /* bind irq to this sensor */
>>>> +               data->irq_bind_sensor = index;
>>>> +       }
>>> I don't see why this should be specified in the DT. Why do you believe
>>> it should?
>> The thermal sensor module has four sensors, but have only one
>> interrupt signal; This interrupt can only be used by one sensor;
>> So want to use dts to bind the interrupt with one selected sensor.
> That's not all that great, though I'm not exactly sure how the kernel
> would select the best sensor to measure with. It would be good if you
> could talk to the thermal maintainers w.r.t. this.
>
>>>> +static int hisi_thermal_probe(struct platform_device *pdev)
>>>> +{
>>>> +       struct hisi_thermal_data *data;
>>>> +       struct resource *res;
>>>> +       int i;
>>>> +       int ret;
>>>> +
>>>> +       if (!cpufreq_get_current_driver()) {
>>>> +               dev_dbg(&pdev->dev, "no cpufreq driver!");
>>>> +               return -EPROBE_DEFER;
>>>> +       }
>>> Surely we care about not burning out the board even if we don't have
>>> cpufreq?
>>>
>>> Is there any ordering guarantee between the probing of this driver and
>>> cpufreq?
>> Yes, here need binding the thermal sensor w/t cpu cooling device,
>> and cpu cooling device is based on cpufreq driver.
> Sure, but if you don't have a cooling device you still want the critical
> temperature reset and so on, no?
>
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Mark Rutland]

> > That may be the case in the code as it stands today, but per the binding
> > the trip points are the temperatures at which an action is to be taken.
> > 
> > The thermal-zone has poilling-delay and polling-delay-passive, but
> > there's no reason you couldn't also use the interrupt to handle the
> > "hot" trip-point, adn the reset at the "critical" trip-point. All that's
> > missing is the plumbing in order to do so.
> > 
> > So please co-ordinate with the thermal framework to do that.
> 
> Let's dig further more for this point, so that we can get more specific
> gudiance and have a good preparation for next version's patch set.
> 
> After i reviewed the thermal framework code, currently have one smooth
> way to co-ordinate the trip points w/t thermal framework: use the function
> *thermal_zone_device_register()* to register sensor, and can use the
> callback function .get_trip_temp to tell thermal framework for the
> trip points' temperature.
> 
> For hisi thermal case, now the driver is using the function
> *thermal_zone_of_sensor_register* to register sensor, but use this way
> i have not found there have standard APIs which can be used by sensor
> driver to get the trip points info from thermal framework.
> 
> I may miss something for thermal framework, so if have existed APIs to
> get the trip points, could pls point out?

I am only familiar with the binding, not the Linux implementation -- The
latter can change to accomodate your hardware without requiring binding
changes. Please co-ordinate with the thermal maintainers.

> > > > > +       if (of_property_read_bool(np, "hisilicon,tsensor-bind-irq")) {
> > > > > +
> > > > > +               if (data->irq_bind_sensor != -1)
> > > > > +                       dev_warn(&pdev->dev, "irq has bound to index %d\n",
> > > > > +                                data->irq_bind_sensor);
> > > > > +
> > > > > +               /* bind irq to this sensor */
> > > > > +               data->irq_bind_sensor = index;
> > > > > +       }
> > > > 
> > > > I don't see why this should be specified in the DT. Why do you believe
> > > > it should?
> > > 
> > > The thermal sensor module has four sensors, but have only one
> > > interrupt signal; This interrupt can only be used by one sensor;
> > > So want to use dts to bind the interrupt with one selected sensor.
> > 
> > That's not all that great, though I'm not exactly sure how the kernel
> > would select the best sensor to measure with. It would be good if you
> > could talk to the thermal maintainers w.r.t. this.
> 
> This will be decided by the silicon, right? Every soc has different
> combination with cpu/gpu/vpu, so which part is hottest, this maybe
> highly dependent on individual SoC.
> 
> S/W just need provide the flexibility so that later can choose
> the interrupt to bind with the sensor within the hottest part.

Then the property you care about is which sensor is closest to what is
likely to be the hottest component. Given that, the kernel can decide
how to use the interrupt.

Thanks,
Mark.

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

[Leo Yan]

On Fri, Mar 20, 2015 at 03:55:27PM +0000, Mark Rutland wrote:
> > > That may be the case in the code as it stands today, but per the binding
> > > the trip points are the temperatures at which an action is to be taken.
> > > 
> > > The thermal-zone has poilling-delay and polling-delay-passive, but
> > > there's no reason you couldn't also use the interrupt to handle the
> > > "hot" trip-point, adn the reset at the "critical" trip-point. All that's
> > > missing is the plumbing in order to do so.
> > > 
> > > So please co-ordinate with the thermal framework to do that.
> > 
> > Let's dig further more for this point, so that we can get more specific
> > gudiance and have a good preparation for next version's patch set.
> > 
> > After i reviewed the thermal framework code, currently have one smooth
> > way to co-ordinate the trip points w/t thermal framework: use the function
> > *thermal_zone_device_register()* to register sensor, and can use the
> > callback function .get_trip_temp to tell thermal framework for the
> > trip points' temperature.
> > 
> > For hisi thermal case, now the driver is using the function
> > *thermal_zone_of_sensor_register* to register sensor, but use this way
> > i have not found there have standard APIs which can be used by sensor
> > driver to get the trip points info from thermal framework.
> > 
> > I may miss something for thermal framework, so if have existed APIs to
> > get the trip points, could pls point out?
> 
> I am only familiar with the binding, not the Linux implementation -- The
> latter can change to accomodate your hardware without requiring binding
> changes. Please co-ordinate with the thermal maintainers.

Found the functions of_thermal_get_trip_points(tz) and of_thermal_get_ntrips(tz)
will help for this.

> > > > > > +       if (of_property_read_bool(np, "hisilicon,tsensor-bind-irq")) {
> > > > > > +
> > > > > > +               if (data->irq_bind_sensor != -1)
> > > > > > +                       dev_warn(&pdev->dev, "irq has bound to index %d\n",
> > > > > > +                                data->irq_bind_sensor);
> > > > > > +
> > > > > > +               /* bind irq to this sensor */
> > > > > > +               data->irq_bind_sensor = index;
> > > > > > +       }
> > > > > 
> > > > > I don't see why this should be specified in the DT. Why do you believe
> > > > > it should?
> > > > 
> > > > The thermal sensor module has four sensors, but have only one
> > > > interrupt signal; This interrupt can only be used by one sensor;
> > > > So want to use dts to bind the interrupt with one selected sensor.
> > > 
> > > That's not all that great, though I'm not exactly sure how the kernel
> > > would select the best sensor to measure with. It would be good if you
> > > could talk to the thermal maintainers w.r.t. this.
> > 
> > This will be decided by the silicon, right? Every soc has different
> > combination with cpu/gpu/vpu, so which part is hottest, this maybe
> > highly dependent on individual SoC.
> > 
> > S/W just need provide the flexibility so that later can choose
> > the interrupt to bind with the sensor within the hottest part.
> 
> Then the property you care about is which sensor is closest to what is
> likely to be the hottest component. Given that, the kernel can decide
> how to use the interrupt.

Will modify the driver to dynamically bind the interrupt to hottest
sensor; Appreciate for good suggestion.

Thanks,
Leo Yan

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

[kongxinwei]

On 03/23/2015 12:46 PM, Leo Yan wrote:
> On Fri, Mar 20, 2015 at 03:55:27PM +0000, Mark Rutland wrote:
>>>> That may be the case in the code as it stands today, but per the binding
>>>> the trip points are the temperatures at which an action is to be taken.
>>>>
>>>> The thermal-zone has poilling-delay and polling-delay-passive, but
>>>> there's no reason you couldn't also use the interrupt to handle the
>>>> "hot" trip-point, adn the reset at the "critical" trip-point. All that's
>>>> missing is the plumbing in order to do so.
>>>>
>>>> So please co-ordinate with the thermal framework to do that.
>>> Let's dig further more for this point, so that we can get more specific
>>> gudiance and have a good preparation for next version's patch set.
>>>
>>> After i reviewed the thermal framework code, currently have one smooth
>>> way to co-ordinate the trip points w/t thermal framework: use the function
>>> *thermal_zone_device_register()* to register sensor, and can use the
>>> callback function .get_trip_temp to tell thermal framework for the
>>> trip points' temperature.
>>>
>>> For hisi thermal case, now the driver is using the function
>>> *thermal_zone_of_sensor_register* to register sensor, but use this way
>>> i have not found there have standard APIs which can be used by sensor
>>> driver to get the trip points info from thermal framework.
>>>
>>> I may miss something for thermal framework, so if have existed APIs to
>>> get the trip points, could pls point out?
>> I am only familiar with the binding, not the Linux implementation -- The
>> latter can change to accomodate your hardware without requiring binding
>> changes. Please co-ordinate with the thermal maintainers.
> Found the functions of_thermal_get_trip_points(tz) and of_thermal_get_ntrips(tz)
> will help for this.
>
>>>>>>> +       if (of_property_read_bool(np, "hisilicon,tsensor-bind-irq")) {
>>>>>>> +
>>>>>>> +               if (data->irq_bind_sensor != -1)
>>>>>>> +                       dev_warn(&pdev->dev, "irq has bound to index %d\n",
>>>>>>> +                                data->irq_bind_sensor);
>>>>>>> +
>>>>>>> +               /* bind irq to this sensor */
>>>>>>> +               data->irq_bind_sensor = index;
>>>>>>> +       }
>>>>>> I don't see why this should be specified in the DT. Why do you believe
>>>>>> it should?
>>>>> The thermal sensor module has four sensors, but have only one
>>>>> interrupt signal; This interrupt can only be used by one sensor;
>>>>> So want to use dts to bind the interrupt with one selected sensor.
>>>> That's not all that great, though I'm not exactly sure how the kernel
>>>> would select the best sensor to measure with. It would be good if you
>>>> could talk to the thermal maintainers w.r.t. this.
>>> This will be decided by the silicon, right? Every soc has different
>>> combination with cpu/gpu/vpu, so which part is hottest, this maybe
>>> highly dependent on individual SoC.
>>>
>>> S/W just need provide the flexibility so that later can choose
>>> the interrupt to bind with the sensor within the hottest part.
>> Then the property you care about is which sensor is closest to what is
>> likely to be the hottest component. Given that, the kernel can decide
>> how to use the interrupt.
> Will modify the driver to dynamically bind the interrupt to hottest
> sensor; Appreciate for good suggestion.
>
> Thanks,
> Leo Yan

Hi mark:
    Thank you for your comments,please wait the next version to slove your
presenting problem.

Thanks.
Xinwei

> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html