[PATCH V2 0/4] thermal/drivers/hisi: Add hi3660 thermal driver support

[PATCH V2 0/4] thermal/drivers/hisi: Add hi3660 thermal driver support

[Daniel Lezcano]

This series adds the support for the hikey960 thermal driver. The tsensor
is almost the same than the hi6220 but with a memory shared updated by a
Cortex-M4.

In order to handle both, the hi6220 and the hi3660, the current code should
be prefixed with the platform name as we will add functions with the same
purpose but slightly different. In addition, the core driver code is
maintained untouched by using a set of ops filled at boot time.

Changelog:

 - V2 : 
        - Fixed extra carriage return, description length reported
          by checkpatch
        - Removed the multi-threshold interrupt
        - Removed DT bindings documentation and DT change as it is
          merge in the arm-soc tree now
        - Tested on hi6220

 - V1 : initial post

Kevin Wangtao (4):
  thermal/drivers/hisi: Put platform code together
  thermal/drivers/hisi: Add platform prefix to function name
  thermal/drivers/hisi: Prepare to add support for other hisi platforms
  thermal/drivers/hisi: Add support for hi3660 SoC

 drivers/thermal/hisi_thermal.c | 452 +++++++++++++++++++++++++++++------------
 1 file changed, 319 insertions(+), 133 deletions(-)

-- 
2.7.4

[PATCH V2 1/4] thermal/drivers/hisi: Put platform code together

[Daniel Lezcano]

From: Kevin Wangtao <kevin.wangtao@linaro.org>

Reorganize the code for next patches by moving the functions upper in
the file which will prevent a forward declaration. There is no functional
change here.

Signed-off-by: Kevin Wangtao <kevin.wangtao@linaro.org>
Tested-by: Daniel Lezcano <daniel.lezcano@linaro.org> # hikey6220
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/hisi_thermal.c | 76 +++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
index b48e47e..ffa00b4 100644
--- a/drivers/thermal/hisi_thermal.c
+++ b/drivers/thermal/hisi_thermal.c
@@ -201,6 +201,44 @@ static void hisi_thermal_disable_sensor(struct hisi_thermal_data *data)
 	clk_disable_unprepare(data->clk);
 }
 
+static int hisi_thermal_setup(struct hisi_thermal_data *data)
+{
+	struct hisi_thermal_sensor *sensor = &data->sensor;
+	int ret;
+
+	/* enable clock for tsensor */
+	ret = clk_prepare_enable(data->clk);
+	if (ret)
+		return ret;
+
+	/* disable module firstly */
+	hisi_thermal_reset_enable(data->regs, 0);
+	hisi_thermal_enable(data->regs, 0);
+
+	/* select sensor id */
+	hisi_thermal_sensor_select(data->regs, sensor->id);
+
+	/* setting the hdak time */
+	hisi_thermal_hdak_set(data->regs, 0);
+
+	/* setting lag value between current temp and the threshold */
+	hisi_thermal_set_lag(data->regs, HISI_TEMP_LAG);
+
+	/* enable for interrupt */
+	hisi_thermal_alarm_set(data->regs, sensor->thres_temp);
+
+	hisi_thermal_reset_set(data->regs, HISI_TEMP_RESET);
+
+	/* enable module */
+	hisi_thermal_reset_enable(data->regs, 1);
+	hisi_thermal_enable(data->regs, 1);
+
+	hisi_thermal_alarm_clear(data->regs, 0);
+	hisi_thermal_alarm_enable(data->regs, 1);
+
+	return 0;
+}
+
 static int hisi_thermal_get_temp(void *__data, int *temp)
 {
 	struct hisi_thermal_data *data = __data;
@@ -291,44 +329,6 @@ static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
 		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
 }
 
-static int hisi_thermal_setup(struct hisi_thermal_data *data)
-{
-	struct hisi_thermal_sensor *sensor = &data->sensor;
-	int ret;
-
-	/* enable clock for tsensor */
-	ret = clk_prepare_enable(data->clk);
-	if (ret)
-		return ret;
-
-	/* disable module firstly */
-	hisi_thermal_reset_enable(data->regs, 0);
-	hisi_thermal_enable(data->regs, 0);
-
-	/* select sensor id */
-	hisi_thermal_sensor_select(data->regs, sensor->id);
-
-	/* setting the hdak time */
-	hisi_thermal_hdak_set(data->regs, 0);
-
-	/* setting lag value between current temp and the threshold */
-	hisi_thermal_set_lag(data->regs, HISI_TEMP_LAG);
-
-	/* enable for interrupt */
-	hisi_thermal_alarm_set(data->regs, sensor->thres_temp);
-
-	hisi_thermal_reset_set(data->regs, HISI_TEMP_RESET);
-
-	/* enable module */
-	hisi_thermal_reset_enable(data->regs, 1);
-	hisi_thermal_enable(data->regs, 1);
-
-	hisi_thermal_alarm_clear(data->regs, 0);
-	hisi_thermal_alarm_enable(data->regs, 1);
-
-	return 0;
-}
-
 static int hisi_thermal_probe(struct platform_device *pdev)
 {
 	struct hisi_thermal_data *data;
-- 
2.7.4

[PATCH V2 2/4] thermal/drivers/hisi: Add platform prefix to function name

[Daniel Lezcano]

From: Kevin Wangtao <kevin.wangtao@linaro.org>

As the next patches will provide support for the hikey3660's sensor,
several functions with the same purpose but for different platforms will
be introduced.

In order to make a clear distinction between them, let's prefix the
function names with the platform name.

This patch has no functional changes, only name changes.

Signed-off-by: Kevin Wangtao <kevin.wangtao@linaro.org>
Tested-by: Daniel Lezcano <daniel.lezcano@linaro.org> # hikey6220
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/hisi_thermal.c | 145 +++++++++++++++++++++--------------------
 1 file changed, 73 insertions(+), 72 deletions(-)

diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
index ffa00b4..a0c0bb9 100644
--- a/drivers/thermal/hisi_thermal.c
+++ b/drivers/thermal/hisi_thermal.c
@@ -26,25 +26,24 @@
 
 #include "thermal_core.h"
 
-#define TEMP0_LAG			(0x0)
-#define TEMP0_TH			(0x4)
-#define TEMP0_RST_TH			(0x8)
-#define TEMP0_CFG			(0xC)
-#define TEMP0_CFG_SS_MSK		(0xF000)
-#define TEMP0_CFG_HDAK_MSK		(0x30)
-#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			(-60000)
-#define HISI_TEMP_RESET			(100000)
-#define HISI_TEMP_STEP			(785)
-#define HISI_TEMP_LAG			(3500)
-
-#define HISI_MAX_SENSORS		4
-#define HISI_DEFAULT_SENSOR		2
+#define HI6220_TEMP0_LAG			(0x0)
+#define HI6220_TEMP0_TH				(0x4)
+#define HI6220_TEMP0_RST_TH			(0x8)
+#define HI6220_TEMP0_CFG			(0xC)
+#define HI6220_TEMP0_CFG_SS_MSK		(0xF000)
+#define HI6220_TEMP0_CFG_HDAK_MSK		(0x30)
+#define HI6220_TEMP0_EN				(0x10)
+#define HI6220_TEMP0_INT_EN			(0x14)
+#define HI6220_TEMP0_INT_CLR			(0x18)
+#define HI6220_TEMP0_RST_MSK			(0x1C)
+#define HI6220_TEMP0_VALUE			(0x28)
+
+#define HI6220_TEMP_BASE			(-60000)
+#define HI6220_TEMP_RESET			(100000)
+#define HI6220_TEMP_STEP			(785)
+#define HI6220_TEMP_LAG			(3500)
+
+#define HI6220_DEFAULT_SENSOR		2
 
 struct hisi_thermal_sensor {
 	struct thermal_zone_device *tzd;
@@ -78,14 +77,14 @@ struct hisi_thermal_data {
  *	steps = (Temp - TempBase) / 785
  *
  */
-static inline int hisi_thermal_step_to_temp(int step)
+static inline int hi6220_thermal_step_to_temp(int step)
 {
-	return HISI_TEMP_BASE + (step * HISI_TEMP_STEP);
+	return HI6220_TEMP_BASE + (step * HI6220_TEMP_STEP);
 }
 
-static inline int hisi_thermal_temp_to_step(int temp)
+static inline int hi6220_thermal_temp_to_step(int temp)
 {
-	return DIV_ROUND_UP(temp - HISI_TEMP_BASE, HISI_TEMP_STEP);
+	return DIV_ROUND_UP(temp - HI6220_TEMP_BASE, HI6220_TEMP_STEP);
 }
 
 /*
@@ -112,51 +111,53 @@ static inline int hisi_thermal_temp_to_step(int temp)
  *
  * [0:4] : lag register
  *
- * The temperature is coded in steps, cf. HISI_TEMP_STEP.
+ * The temperature is coded in steps, cf. HI6220_TEMP_STEP.
  *
  * Min : 0x00 :  0.0 °C
  * Max : 0x1F : 24.3 °C
  *
  * The 'value' parameter is in milliCelsius.
  */
-static inline void hisi_thermal_set_lag(void __iomem *addr, int value)
+static inline void hi6220_thermal_set_lag(void __iomem *addr, int value)
 {
-	writel(DIV_ROUND_UP(value, HISI_TEMP_STEP) & 0x1F, addr + TEMP0_LAG);
+	writel(DIV_ROUND_UP(value, HI6220_TEMP_STEP) & 0x1F,
+			addr + HI6220_TEMP0_LAG);
 }
 
-static inline void hisi_thermal_alarm_clear(void __iomem *addr, int value)
+static inline void hi6220_thermal_alarm_clear(void __iomem *addr, int value)
 {
-	writel(value, addr + TEMP0_INT_CLR);
+	writel(value, addr + HI6220_TEMP0_INT_CLR);
 }
 
-static inline void hisi_thermal_alarm_enable(void __iomem *addr, int value)
+static inline void hi6220_thermal_alarm_enable(void __iomem *addr, int value)
 {
-	writel(value, addr + TEMP0_INT_EN);
+	writel(value, addr + HI6220_TEMP0_INT_EN);
 }
 
-static inline void hisi_thermal_alarm_set(void __iomem *addr, int temp)
+static inline void hi6220_thermal_alarm_set(void __iomem *addr, int temp)
 {
-	writel(hisi_thermal_temp_to_step(temp) | 0x0FFFFFF00, addr + TEMP0_TH);
+	writel(hi6220_thermal_temp_to_step(temp) | 0x0FFFFFF00,
+	       addr + HI6220_TEMP0_TH);
 }
 
-static inline void hisi_thermal_reset_set(void __iomem *addr, int temp)
+static inline void hi6220_thermal_reset_set(void __iomem *addr, int temp)
 {
-	writel(hisi_thermal_temp_to_step(temp), addr + TEMP0_RST_TH);
+	writel(hi6220_thermal_temp_to_step(temp), addr + HI6220_TEMP0_RST_TH);
 }
 
-static inline void hisi_thermal_reset_enable(void __iomem *addr, int value)
+static inline void hi6220_thermal_reset_enable(void __iomem *addr, int value)
 {
-	writel(value, addr + TEMP0_RST_MSK);
+	writel(value, addr + HI6220_TEMP0_RST_MSK);
 }
 
-static inline void hisi_thermal_enable(void __iomem *addr, int value)
+static inline void hi6220_thermal_enable(void __iomem *addr, int value)
 {
-	writel(value, addr + TEMP0_EN);
+	writel(value, addr + HI6220_TEMP0_EN);
 }
 
-static inline int hisi_thermal_get_temperature(void __iomem *addr)
+static inline int hi6220_thermal_get_temperature(void __iomem *addr)
 {
-	return hisi_thermal_step_to_temp(readl(addr + TEMP0_VALUE));
+	return hi6220_thermal_step_to_temp(readl(addr + HI6220_TEMP0_VALUE));
 }
 
 /*
@@ -169,10 +170,10 @@ static inline int hisi_thermal_get_temperature(void __iomem *addr)
  * 0x2: remote sensor 2 (ACPU cluster 0)
  * 0x3: remote sensor 3 (G3D)
  */
-static inline void hisi_thermal_sensor_select(void __iomem *addr, int sensor)
+static inline void hi6220_thermal_sensor_select(void __iomem *addr, int sensor)
 {
-	writel((readl(addr + TEMP0_CFG) & ~TEMP0_CFG_SS_MSK) |
-	       (sensor << 12), addr + TEMP0_CFG);
+	writel((readl(addr + HI6220_TEMP0_CFG) & ~HI6220_TEMP0_CFG_SS_MSK) |
+	       (sensor << 12), addr + HI6220_TEMP0_CFG);
 }
 
 /*
@@ -185,23 +186,23 @@ static inline void hisi_thermal_sensor_select(void __iomem *addr, int sensor)
  * 0x2 :  49.152 ms
  * 0x3 : 393.216 ms
  */
-static inline void hisi_thermal_hdak_set(void __iomem *addr, int value)
+static inline void hi6220_thermal_hdak_set(void __iomem *addr, int value)
 {
-	writel((readl(addr + TEMP0_CFG) & ~TEMP0_CFG_HDAK_MSK) |
-	       (value << 4), addr + TEMP0_CFG);
+	writel((readl(addr + HI6220_TEMP0_CFG) & ~HI6220_TEMP0_CFG_HDAK_MSK) |
+	       (value << 4), addr + HI6220_TEMP0_CFG);
 }
 
-static void hisi_thermal_disable_sensor(struct hisi_thermal_data *data)
+static void hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
 {
 	/* disable sensor module */
-	hisi_thermal_enable(data->regs, 0);
-	hisi_thermal_alarm_enable(data->regs, 0);
-	hisi_thermal_reset_enable(data->regs, 0);
+	hi6220_thermal_enable(data->regs, 0);
+	hi6220_thermal_alarm_enable(data->regs, 0);
+	hi6220_thermal_reset_enable(data->regs, 0);
 
 	clk_disable_unprepare(data->clk);
 }
 
-static int hisi_thermal_setup(struct hisi_thermal_data *data)
+static int hi6220_thermal_enable_sensor(struct hisi_thermal_data *data)
 {
 	struct hisi_thermal_sensor *sensor = &data->sensor;
 	int ret;
@@ -212,29 +213,29 @@ static int hisi_thermal_setup(struct hisi_thermal_data *data)
 		return ret;
 
 	/* disable module firstly */
-	hisi_thermal_reset_enable(data->regs, 0);
-	hisi_thermal_enable(data->regs, 0);
+	hi6220_thermal_reset_enable(data->regs, 0);
+	hi6220_thermal_enable(data->regs, 0);
 
 	/* select sensor id */
-	hisi_thermal_sensor_select(data->regs, sensor->id);
+	hi6220_thermal_sensor_select(data->regs, sensor->id);
 
 	/* setting the hdak time */
-	hisi_thermal_hdak_set(data->regs, 0);
+	hi6220_thermal_hdak_set(data->regs, 0);
 
 	/* setting lag value between current temp and the threshold */
-	hisi_thermal_set_lag(data->regs, HISI_TEMP_LAG);
+	hi6220_thermal_set_lag(data->regs, HI6220_TEMP_LAG);
 
 	/* enable for interrupt */
-	hisi_thermal_alarm_set(data->regs, sensor->thres_temp);
+	hi6220_thermal_alarm_set(data->regs, sensor->thres_temp);
 
-	hisi_thermal_reset_set(data->regs, HISI_TEMP_RESET);
+	hi6220_thermal_reset_set(data->regs, HI6220_TEMP_RESET);
 
 	/* enable module */
-	hisi_thermal_reset_enable(data->regs, 1);
-	hisi_thermal_enable(data->regs, 1);
+	hi6220_thermal_reset_enable(data->regs, 1);
+	hi6220_thermal_enable(data->regs, 1);
 
-	hisi_thermal_alarm_clear(data->regs, 0);
-	hisi_thermal_alarm_enable(data->regs, 1);
+	hi6220_thermal_alarm_clear(data->regs, 0);
+	hi6220_thermal_alarm_enable(data->regs, 1);
 
 	return 0;
 }
@@ -244,7 +245,7 @@ static int hisi_thermal_get_temp(void *__data, int *temp)
 	struct hisi_thermal_data *data = __data;
 	struct hisi_thermal_sensor *sensor = &data->sensor;
 
-	*temp = hisi_thermal_get_temperature(data->regs);
+	*temp = hi6220_thermal_get_temperature(data->regs);
 
 	dev_dbg(&data->pdev->dev, "id=%d, temp=%d, thres=%d\n",
 		sensor->id, *temp, sensor->thres_temp);
@@ -260,11 +261,11 @@ static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
 {
 	struct hisi_thermal_data *data = dev;
 	struct hisi_thermal_sensor *sensor = &data->sensor;
-	int temp;
+	int temp = 0;
 
-	hisi_thermal_alarm_clear(data->regs, 1);
+	hi6220_thermal_alarm_clear(data->regs, 1);
 
-	temp = hisi_thermal_get_temperature(data->regs);
+	hisi_thermal_get_temp(data, &temp);
 
 	if (temp >= sensor->thres_temp) {
 		dev_crit(&data->pdev->dev, "THERMAL ALARM: %d > %d\n",
@@ -273,7 +274,7 @@ static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
 		thermal_zone_device_update(data->sensor.tzd,
 					   THERMAL_EVENT_UNSPECIFIED);
 
-	} else if (temp < sensor->thres_temp) {
+	} else {
 		dev_crit(&data->pdev->dev, "THERMAL ALARM stopped: %d < %d\n",
 			 temp, sensor->thres_temp);
 	}
@@ -365,14 +366,14 @@ static int hisi_thermal_probe(struct platform_device *pdev)
 
 	ret = hisi_thermal_register_sensor(pdev, data,
 					   &data->sensor,
-					   HISI_DEFAULT_SENSOR);
+					   HI6220_DEFAULT_SENSOR);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register thermal sensor: %d\n",
 			ret);
 		return ret;
 	}
 
-	ret = hisi_thermal_setup(data);
+	ret = hi6220_thermal_enable_sensor(data);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to setup the sensor: %d\n", ret);
 		return ret;
@@ -397,7 +398,7 @@ static int hisi_thermal_remove(struct platform_device *pdev)
 	struct hisi_thermal_sensor *sensor = &data->sensor;
 
 	hisi_thermal_toggle_sensor(sensor, false);
-	hisi_thermal_disable_sensor(data);
+	hi6220_thermal_disable_sensor(data);
 
 	return 0;
 }
@@ -407,7 +408,7 @@ static int hisi_thermal_suspend(struct device *dev)
 {
 	struct hisi_thermal_data *data = dev_get_drvdata(dev);
 
-	hisi_thermal_disable_sensor(data);
+	hi6220_thermal_disable_sensor(data);
 
 	return 0;
 }
@@ -416,7 +417,7 @@ static int hisi_thermal_resume(struct device *dev)
 {
 	struct hisi_thermal_data *data = dev_get_drvdata(dev);
 
-	return hisi_thermal_setup(data);
+	return hi6220_thermal_enable_sensor(data);
 }
 #endif
 
-- 
2.7.4

[PATCH V2 3/4] thermal/drivers/hisi: Prepare to add support for other hisi platforms

[Daniel Lezcano]

From: Kevin Wangtao <kevin.wangtao@linaro.org>

For platform compatibility, add the tsensor ops to a thermal data
structure. Each platform has its own probe function to register proper
tsensor ops function to the pointer, platform related resource request
are also implemented in the platform probe function.

Signed-off-by: Kevin Wangtao <kevin.wangtao@linaro.org>
Tested-by: Daniel Lezcano <daniel.lezcano@linaro.org> # hikey6220
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/hisi_thermal.c | 132 +++++++++++++++++++++++++++--------------
 1 file changed, 87 insertions(+), 45 deletions(-)

diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
index a0c0bb9..98a4403 100644
--- a/drivers/thermal/hisi_thermal.c
+++ b/drivers/thermal/hisi_thermal.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
+#include <linux/of_device.h>
 
 #include "thermal_core.h"
 
@@ -30,7 +31,7 @@
 #define HI6220_TEMP0_TH				(0x4)
 #define HI6220_TEMP0_RST_TH			(0x8)
 #define HI6220_TEMP0_CFG			(0xC)
-#define HI6220_TEMP0_CFG_SS_MSK		(0xF000)
+#define HI6220_TEMP0_CFG_SS_MSK			(0xF000)
 #define HI6220_TEMP0_CFG_HDAK_MSK		(0x30)
 #define HI6220_TEMP0_EN				(0x10)
 #define HI6220_TEMP0_INT_EN			(0x14)
@@ -41,7 +42,7 @@
 #define HI6220_TEMP_BASE			(-60000)
 #define HI6220_TEMP_RESET			(100000)
 #define HI6220_TEMP_STEP			(785)
-#define HI6220_TEMP_LAG			(3500)
+#define HI6220_TEMP_LAG				(3500)
 
 #define HI6220_DEFAULT_SENSOR		2
 
@@ -52,6 +53,10 @@ struct hisi_thermal_sensor {
 };
 
 struct hisi_thermal_data {
+	int (*get_temp)(struct hisi_thermal_data *data);
+	int (*enable_sensor)(struct hisi_thermal_data *data);
+	int (*disable_sensor)(struct hisi_thermal_data *data);
+	int (*irq_handler)(struct hisi_thermal_data *data);
 	struct platform_device *pdev;
 	struct clk *clk;
 	struct hisi_thermal_sensor sensor;
@@ -192,7 +197,18 @@ static inline void hi6220_thermal_hdak_set(void __iomem *addr, int value)
 	       (value << 4), addr + HI6220_TEMP0_CFG);
 }
 
-static void hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
+static int hi6220_thermal_irq_handler(struct hisi_thermal_data *data)
+{
+	hi6220_thermal_alarm_clear(data->regs, 1);
+	return 0;
+}
+
+static int hi6220_thermal_get_temp(struct hisi_thermal_data *data)
+{
+	return hi6220_thermal_get_temperature(data->regs);
+}
+
+static int hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
 {
 	/* disable sensor module */
 	hi6220_thermal_enable(data->regs, 0);
@@ -200,6 +216,8 @@ static void hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
 	hi6220_thermal_reset_enable(data->regs, 0);
 
 	clk_disable_unprepare(data->clk);
+
+	return 0;
 }
 
 static int hi6220_thermal_enable_sensor(struct hisi_thermal_data *data)
@@ -240,12 +258,48 @@ static int hi6220_thermal_enable_sensor(struct hisi_thermal_data *data)
 	return 0;
 }
 
+static int hi6220_thermal_probe(struct hisi_thermal_data *data)
+{
+	struct platform_device *pdev = data->pdev;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	int ret;
+
+	data->get_temp = hi6220_thermal_get_temp;
+	data->enable_sensor = hi6220_thermal_enable_sensor;
+	data->disable_sensor = hi6220_thermal_disable_sensor;
+	data->irq_handler = hi6220_thermal_irq_handler;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(data->regs)) {
+		dev_err(dev, "failed to get io address\n");
+		return PTR_ERR(data->regs);
+	}
+
+	data->clk = devm_clk_get(dev, "thermal_clk");
+	if (IS_ERR(data->clk)) {
+		ret = PTR_ERR(data->clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to get thermal clk: %d\n", ret);
+		return ret;
+	}
+
+	data->irq = platform_get_irq(pdev, 0);
+	if (data->irq < 0)
+		return data->irq;
+
+	data->sensor.id = HI6220_DEFAULT_SENSOR;
+
+	return 0;
+}
+
 static int hisi_thermal_get_temp(void *__data, int *temp)
 {
 	struct hisi_thermal_data *data = __data;
 	struct hisi_thermal_sensor *sensor = &data->sensor;
 
-	*temp = hi6220_thermal_get_temperature(data->regs);
+	*temp = data->get_temp(data);
 
 	dev_dbg(&data->pdev->dev, "id=%d, temp=%d, thres=%d\n",
 		sensor->id, *temp, sensor->thres_temp);
@@ -263,7 +317,7 @@ static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
 	struct hisi_thermal_sensor *sensor = &data->sensor;
 	int temp = 0;
 
-	hi6220_thermal_alarm_clear(data->regs, 1);
+	data->irq_handler(data);
 
 	hisi_thermal_get_temp(data, &temp);
 
@@ -284,14 +338,11 @@ static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
 
 static int hisi_thermal_register_sensor(struct platform_device *pdev,
 					struct hisi_thermal_data *data,
-					struct hisi_thermal_sensor *sensor,
-					int index)
+					struct hisi_thermal_sensor *sensor)
 {
 	int ret, i;
 	const struct thermal_trip *trip;
 
-	sensor->id = index;
-
 	sensor->tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
 							   sensor->id, data,
 							   &hisi_of_thermal_ops);
@@ -316,7 +367,7 @@ static int hisi_thermal_register_sensor(struct platform_device *pdev,
 }
 
 static const struct of_device_id of_hisi_thermal_match[] = {
-	{ .compatible = "hisilicon,tsensor" },
+	{ .compatible = "hisilicon,tsensor", .data = hi6220_thermal_probe },
 	{ /* end */ }
 };
 MODULE_DEVICE_TABLE(of, of_hisi_thermal_match);
@@ -333,58 +384,48 @@ static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
 static int hisi_thermal_probe(struct platform_device *pdev)
 {
 	struct hisi_thermal_data *data;
-	struct resource *res;
+	int (*platform_probe)(struct hisi_thermal_data *);
+	struct device *dev = &pdev->dev;
 	int ret;
 
-	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
 	data->pdev = pdev;
+	platform_set_drvdata(pdev, data);
 
-	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);
+	platform_probe = of_device_get_match_data(dev);
+	if (!platform_probe) {
+		dev_err(dev, "failed to get probe func\n");
+		return -EINVAL;
 	}
 
-	data->irq = platform_get_irq(pdev, 0);
-	if (data->irq < 0)
-		return data->irq;
-
-	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);
+	ret = platform_probe(data);
+	if (ret)
 		return ret;
-	}
 
 	ret = hisi_thermal_register_sensor(pdev, data,
-					   &data->sensor,
-					   HI6220_DEFAULT_SENSOR);
+					   &data->sensor);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to register thermal sensor: %d\n",
-			ret);
+		dev_err(dev, "failed to register thermal sensor: %d\n", ret);
 		return ret;
 	}
 
-	ret = hi6220_thermal_enable_sensor(data);
+	ret = data->enable_sensor(data);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to setup the sensor: %d\n", ret);
+		dev_err(dev, "Failed to setup the sensor: %d\n", ret);
 		return ret;
 	}
 
-	ret = devm_request_threaded_irq(&pdev->dev, data->irq, NULL,
-					hisi_thermal_alarm_irq_thread,
-					IRQF_ONESHOT, "hisi_thermal", data);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to request alarm irq: %d\n", ret);
-		return ret;
+	if (data->irq) {
+		ret = devm_request_threaded_irq(dev, data->irq, NULL,
+				hisi_thermal_alarm_irq_thread,
+				IRQF_ONESHOT, "hisi_thermal", data);
+		if (ret < 0) {
+			dev_err(dev, "failed to request alarm irq: %d\n", ret);
+			return ret;
+		}
 	}
 
 	hisi_thermal_toggle_sensor(&data->sensor, true);
@@ -398,7 +439,8 @@ static int hisi_thermal_remove(struct platform_device *pdev)
 	struct hisi_thermal_sensor *sensor = &data->sensor;
 
 	hisi_thermal_toggle_sensor(sensor, false);
-	hi6220_thermal_disable_sensor(data);
+
+	data->disable_sensor(data);
 
 	return 0;
 }
@@ -408,7 +450,7 @@ static int hisi_thermal_suspend(struct device *dev)
 {
 	struct hisi_thermal_data *data = dev_get_drvdata(dev);
 
-	hi6220_thermal_disable_sensor(data);
+	data->disable_sensor(data);
 
 	return 0;
 }
@@ -417,7 +459,7 @@ static int hisi_thermal_resume(struct device *dev)
 {
 	struct hisi_thermal_data *data = dev_get_drvdata(dev);
 
-	return hi6220_thermal_enable_sensor(data);
+	return data->enable_sensor(data);
 }
 #endif
 
-- 
2.7.4

[PATCH V2 4/4] thermal/drivers/hisi: Add support for hi3660 SoC

[Daniel Lezcano]

From: Kevin Wangtao <kevin.wangtao@linaro.org>

This patch adds the support for thermal sensor on the Hi3660 SoC.
Hi3660 tsensor support alarm in alarm threshold, it also has a configurable
hysteresis interval, interrupt will be triggered when temperature rise above
the alarm threshold or fall below the hysteresis threshold.

Signed-off-by: Kevin Wangtao <kevin.wangtao@linaro.org>
Tested-by: Daniel Lezcano <daniel.lezcano@linaro.org> # hikey6220
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/hisi_thermal.c | 145 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 144 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
index 98a4403..94186e7 100644
--- a/drivers/thermal/hisi_thermal.c
+++ b/drivers/thermal/hisi_thermal.c
@@ -39,12 +39,24 @@
 #define HI6220_TEMP0_RST_MSK			(0x1C)
 #define HI6220_TEMP0_VALUE			(0x28)
 
+#define HI3660_OFFSET(chan)		((chan) * 0x40)
+#define HI3660_TEMP(chan)		(HI3660_OFFSET(chan) + 0x1C)
+#define HI3660_TH(chan)			(HI3660_OFFSET(chan) + 0x20)
+#define HI3660_LAG(chan)		(HI3660_OFFSET(chan) + 0x28)
+#define HI3660_INT_EN(chan)		(HI3660_OFFSET(chan) + 0x2C)
+#define HI3660_INT_CLR(chan)		(HI3660_OFFSET(chan) + 0x30)
+
 #define HI6220_TEMP_BASE			(-60000)
 #define HI6220_TEMP_RESET			(100000)
 #define HI6220_TEMP_STEP			(785)
 #define HI6220_TEMP_LAG				(3500)
 
+#define HI3660_TEMP_BASE		(-63780)
+#define HI3660_TEMP_STEP		(205)
+#define HI3660_TEMP_LAG			(4000)
+
 #define HI6220_DEFAULT_SENSOR		2
+#define HI3660_DEFAULT_SENSOR		1
 
 struct hisi_thermal_sensor {
 	struct thermal_zone_device *tzd;
@@ -93,6 +105,24 @@ static inline int hi6220_thermal_temp_to_step(int temp)
 }
 
 /*
+ * for Hi3660,
+ *	Step: 189/922 (0.205)
+ *	Temperature base: -63.780°C
+ *
+ * The register is programmed in temperature steps, every step is 205
+ * millidegree and begins at -63 780 m°C
+ */
+static inline int hi3660_thermal_step_to_temp(int step)
+{
+	return HI3660_TEMP_BASE + step * HI3660_TEMP_STEP;
+}
+
+static inline int hi3660_thermal_temp_to_step(int temp)
+{
+	return DIV_ROUND_UP(temp - HI3660_TEMP_BASE, HI3660_TEMP_STEP);
+}
+
+/*
  * The lag register contains 5 bits encoding the temperature in steps.
  *
  * Each time the temperature crosses the threshold boundary, an
@@ -166,6 +196,45 @@ static inline int hi6220_thermal_get_temperature(void __iomem *addr)
 }
 
 /*
+ * [0:6] lag register
+ *
+ * The temperature is coded in steps, cf. HI3660_TEMP_STEP.
+ *
+ * Min : 0x00 :  0.0 °C
+ * Max : 0x7F : 26.0 °C
+ *
+ */
+static inline void hi3660_thermal_set_lag(void __iomem *addr,
+					  int id, int value)
+{
+	writel(DIV_ROUND_UP(value, HI3660_TEMP_STEP) & 0x7F,
+			addr + HI3660_LAG(id));
+}
+
+static inline void hi3660_thermal_alarm_clear(void __iomem *addr,
+					      int id, int value)
+{
+	writel(value, addr + HI3660_INT_CLR(id));
+}
+
+static inline void hi3660_thermal_alarm_enable(void __iomem *addr,
+					       int id, int value)
+{
+	writel(value, addr + HI3660_INT_EN(id));
+}
+
+static inline void hi3660_thermal_alarm_set(void __iomem *addr,
+					    int id, int value)
+{
+	writel(value, addr + HI3660_TH(id));
+}
+
+static inline int hi3660_thermal_get_temperature(void __iomem *addr, int id)
+{
+	return hi3660_thermal_step_to_temp(readl(addr + HI3660_TEMP(id)));
+}
+
+/*
  * Temperature configuration register - Sensor selection
  *
  * Bits [19:12]
@@ -203,11 +272,22 @@ static int hi6220_thermal_irq_handler(struct hisi_thermal_data *data)
 	return 0;
 }
 
+static int hi3660_thermal_irq_handler(struct hisi_thermal_data *data)
+{
+	hi3660_thermal_alarm_clear(data->regs, data->sensor.id, 1);
+	return 0;
+}
+
 static int hi6220_thermal_get_temp(struct hisi_thermal_data *data)
 {
 	return hi6220_thermal_get_temperature(data->regs);
 }
 
+static int hi3660_thermal_get_temp(struct hisi_thermal_data *data)
+{
+	return hi3660_thermal_get_temperature(data->regs, data->sensor.id);
+}
+
 static int hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
 {
 	/* disable sensor module */
@@ -220,6 +300,13 @@ static int hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
 	return 0;
 }
 
+static int hi3660_thermal_disable_sensor(struct hisi_thermal_data *data)
+{
+	/* disable sensor module */
+	hi3660_thermal_alarm_enable(data->regs, data->sensor.id, 0);
+	return 0;
+}
+
 static int hi6220_thermal_enable_sensor(struct hisi_thermal_data *data)
 {
 	struct hisi_thermal_sensor *sensor = &data->sensor;
@@ -258,6 +345,28 @@ static int hi6220_thermal_enable_sensor(struct hisi_thermal_data *data)
 	return 0;
 }
 
+static int hi3660_thermal_enable_sensor(struct hisi_thermal_data *data)
+{
+	unsigned int value;
+	struct hisi_thermal_sensor *sensor = &data->sensor;
+
+	/* disable interrupt */
+	hi3660_thermal_alarm_enable(data->regs, sensor->id, 0);
+
+	/* setting lag value between current temp and the threshold */
+	hi3660_thermal_set_lag(data->regs, sensor->id, HI3660_TEMP_LAG);
+
+	/* set interrupt threshold */
+	value = hi3660_thermal_temp_to_step(sensor->thres_temp);
+	hi3660_thermal_alarm_set(data->regs, sensor->id, value);
+
+	/* enable interrupt */
+	hi3660_thermal_alarm_clear(data->regs, sensor->id, 1);
+	hi3660_thermal_alarm_enable(data->regs, sensor->id, 1);
+
+	return 0;
+}
+
 static int hi6220_thermal_probe(struct hisi_thermal_data *data)
 {
 	struct platform_device *pdev = data->pdev;
@@ -294,6 +403,33 @@ static int hi6220_thermal_probe(struct hisi_thermal_data *data)
 	return 0;
 }
 
+static int hi3660_thermal_probe(struct hisi_thermal_data *data)
+{
+	struct platform_device *pdev = data->pdev;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+
+	data->get_temp = hi3660_thermal_get_temp;
+	data->enable_sensor = hi3660_thermal_enable_sensor;
+	data->disable_sensor = hi3660_thermal_disable_sensor;
+	data->irq_handler = hi3660_thermal_irq_handler;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(data->regs)) {
+		dev_err(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;
+
+	data->sensor.id = HI3660_DEFAULT_SENSOR;
+
+	return 0;
+}
+
 static int hisi_thermal_get_temp(void *__data, int *temp)
 {
 	struct hisi_thermal_data *data = __data;
@@ -367,7 +503,14 @@ static int hisi_thermal_register_sensor(struct platform_device *pdev,
 }
 
 static const struct of_device_id of_hisi_thermal_match[] = {
-	{ .compatible = "hisilicon,tsensor", .data = hi6220_thermal_probe },
+	{
+		.compatible = "hisilicon,tsensor",
+		.data = hi6220_thermal_probe
+	},
+	{
+		.compatible = "hisilicon,hi3660-tsensor",
+		.data = hi3660_thermal_probe
+	},
 	{ /* end */ }
 };
 MODULE_DEVICE_TABLE(of, of_hisi_thermal_match);
-- 
2.7.4

Re: [PATCH V2 3/4] thermal/drivers/hisi: Prepare to add support for other hisi platforms

[Eduardo Valentin]

On Fri, Oct 20, 2017 at 05:11:06PM +0200, Daniel Lezcano wrote:
> From: Kevin Wangtao <kevin.wangtao@linaro.org>
> 
> For platform compatibility, add the tsensor ops to a thermal data
> structure. Each platform has its own probe function to register proper
> tsensor ops function to the pointer, platform related resource request
> are also implemented in the platform probe function.
> 
> Signed-off-by: Kevin Wangtao <kevin.wangtao@linaro.org>
> Tested-by: Daniel Lezcano <daniel.lezcano@linaro.org> # hikey6220
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

This patch adds this issue to hisi driver (sparse)

drivers/thermal/hisi_thermal.c:398:24: warning: incorrect type in assignment (different modifiers)
drivers/thermal/hisi_thermal.c:398:24:    expected int ( *platform_probe )( ... )
drivers/thermal/hisi_thermal.c:398:24:    got void const *

essentially you are casting a const into a non const.

Please fix and resend.

> ---
>  drivers/thermal/hisi_thermal.c | 132 +++++++++++++++++++++++++++--------------
>  1 file changed, 87 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
> index a0c0bb9..98a4403 100644
> --- a/drivers/thermal/hisi_thermal.c
> +++ b/drivers/thermal/hisi_thermal.c
> @@ -23,6 +23,7 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/io.h>
> +#include <linux/of_device.h>
>  
>  #include "thermal_core.h"
>  
> @@ -30,7 +31,7 @@
>  #define HI6220_TEMP0_TH				(0x4)
>  #define HI6220_TEMP0_RST_TH			(0x8)
>  #define HI6220_TEMP0_CFG			(0xC)
> -#define HI6220_TEMP0_CFG_SS_MSK		(0xF000)
> +#define HI6220_TEMP0_CFG_SS_MSK			(0xF000)
>  #define HI6220_TEMP0_CFG_HDAK_MSK		(0x30)
>  #define HI6220_TEMP0_EN				(0x10)
>  #define HI6220_TEMP0_INT_EN			(0x14)
> @@ -41,7 +42,7 @@
>  #define HI6220_TEMP_BASE			(-60000)
>  #define HI6220_TEMP_RESET			(100000)
>  #define HI6220_TEMP_STEP			(785)
> -#define HI6220_TEMP_LAG			(3500)
> +#define HI6220_TEMP_LAG				(3500)
>  
>  #define HI6220_DEFAULT_SENSOR		2
>  
> @@ -52,6 +53,10 @@ struct hisi_thermal_sensor {
>  };
>  
>  struct hisi_thermal_data {
> +	int (*get_temp)(struct hisi_thermal_data *data);
> +	int (*enable_sensor)(struct hisi_thermal_data *data);
> +	int (*disable_sensor)(struct hisi_thermal_data *data);
> +	int (*irq_handler)(struct hisi_thermal_data *data);
>  	struct platform_device *pdev;
>  	struct clk *clk;
>  	struct hisi_thermal_sensor sensor;
> @@ -192,7 +197,18 @@ static inline void hi6220_thermal_hdak_set(void __iomem *addr, int value)
>  	       (value << 4), addr + HI6220_TEMP0_CFG);
>  }
>  
> -static void hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
> +static int hi6220_thermal_irq_handler(struct hisi_thermal_data *data)
> +{
> +	hi6220_thermal_alarm_clear(data->regs, 1);
> +	return 0;
> +}
> +
> +static int hi6220_thermal_get_temp(struct hisi_thermal_data *data)
> +{
> +	return hi6220_thermal_get_temperature(data->regs);
> +}
> +
> +static int hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
>  {
>  	/* disable sensor module */
>  	hi6220_thermal_enable(data->regs, 0);
> @@ -200,6 +216,8 @@ static void hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
>  	hi6220_thermal_reset_enable(data->regs, 0);
>  
>  	clk_disable_unprepare(data->clk);
> +
> +	return 0;
>  }
>  
>  static int hi6220_thermal_enable_sensor(struct hisi_thermal_data *data)
> @@ -240,12 +258,48 @@ static int hi6220_thermal_enable_sensor(struct hisi_thermal_data *data)
>  	return 0;
>  }
>  
> +static int hi6220_thermal_probe(struct hisi_thermal_data *data)
> +{
> +	struct platform_device *pdev = data->pdev;
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	int ret;
> +
> +	data->get_temp = hi6220_thermal_get_temp;
> +	data->enable_sensor = hi6220_thermal_enable_sensor;
> +	data->disable_sensor = hi6220_thermal_disable_sensor;
> +	data->irq_handler = hi6220_thermal_irq_handler;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	data->regs = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(data->regs)) {
> +		dev_err(dev, "failed to get io address\n");
> +		return PTR_ERR(data->regs);
> +	}
> +
> +	data->clk = devm_clk_get(dev, "thermal_clk");
> +	if (IS_ERR(data->clk)) {
> +		ret = PTR_ERR(data->clk);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get thermal clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	data->irq = platform_get_irq(pdev, 0);
> +	if (data->irq < 0)
> +		return data->irq;
> +
> +	data->sensor.id = HI6220_DEFAULT_SENSOR;
> +
> +	return 0;
> +}
> +
>  static int hisi_thermal_get_temp(void *__data, int *temp)
>  {
>  	struct hisi_thermal_data *data = __data;
>  	struct hisi_thermal_sensor *sensor = &data->sensor;
>  
> -	*temp = hi6220_thermal_get_temperature(data->regs);
> +	*temp = data->get_temp(data);
>  
>  	dev_dbg(&data->pdev->dev, "id=%d, temp=%d, thres=%d\n",
>  		sensor->id, *temp, sensor->thres_temp);
> @@ -263,7 +317,7 @@ static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
>  	struct hisi_thermal_sensor *sensor = &data->sensor;
>  	int temp = 0;
>  
> -	hi6220_thermal_alarm_clear(data->regs, 1);
> +	data->irq_handler(data);
>  
>  	hisi_thermal_get_temp(data, &temp);
>  
> @@ -284,14 +338,11 @@ static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
>  
>  static int hisi_thermal_register_sensor(struct platform_device *pdev,
>  					struct hisi_thermal_data *data,
> -					struct hisi_thermal_sensor *sensor,
> -					int index)
> +					struct hisi_thermal_sensor *sensor)
>  {
>  	int ret, i;
>  	const struct thermal_trip *trip;
>  
> -	sensor->id = index;
> -
>  	sensor->tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
>  							   sensor->id, data,
>  							   &hisi_of_thermal_ops);
> @@ -316,7 +367,7 @@ static int hisi_thermal_register_sensor(struct platform_device *pdev,
>  }
>  
>  static const struct of_device_id of_hisi_thermal_match[] = {
> -	{ .compatible = "hisilicon,tsensor" },
> +	{ .compatible = "hisilicon,tsensor", .data = hi6220_thermal_probe },
>  	{ /* end */ }
>  };
>  MODULE_DEVICE_TABLE(of, of_hisi_thermal_match);
> @@ -333,58 +384,48 @@ static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
>  static int hisi_thermal_probe(struct platform_device *pdev)
>  {
>  	struct hisi_thermal_data *data;
> -	struct resource *res;
> +	int (*platform_probe)(struct hisi_thermal_data *);
> +	struct device *dev = &pdev->dev;
>  	int ret;
>  
> -	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
>  
>  	data->pdev = pdev;
> +	platform_set_drvdata(pdev, data);
>  
> -	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);
> +	platform_probe = of_device_get_match_data(dev);
> +	if (!platform_probe) {
> +		dev_err(dev, "failed to get probe func\n");
> +		return -EINVAL;
>  	}
>  
> -	data->irq = platform_get_irq(pdev, 0);
> -	if (data->irq < 0)
> -		return data->irq;
> -
> -	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);
> +	ret = platform_probe(data);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	ret = hisi_thermal_register_sensor(pdev, data,
> -					   &data->sensor,
> -					   HI6220_DEFAULT_SENSOR);
> +					   &data->sensor);
>  	if (ret) {
> -		dev_err(&pdev->dev, "failed to register thermal sensor: %d\n",
> -			ret);
> +		dev_err(dev, "failed to register thermal sensor: %d\n", ret);
>  		return ret;
>  	}
>  
> -	ret = hi6220_thermal_enable_sensor(data);
> +	ret = data->enable_sensor(data);
>  	if (ret) {
> -		dev_err(&pdev->dev, "Failed to setup the sensor: %d\n", ret);
> +		dev_err(dev, "Failed to setup the sensor: %d\n", ret);
>  		return ret;
>  	}
>  
> -	ret = devm_request_threaded_irq(&pdev->dev, data->irq, NULL,
> -					hisi_thermal_alarm_irq_thread,
> -					IRQF_ONESHOT, "hisi_thermal", data);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "failed to request alarm irq: %d\n", ret);
> -		return ret;
> +	if (data->irq) {
> +		ret = devm_request_threaded_irq(dev, data->irq, NULL,
> +				hisi_thermal_alarm_irq_thread,
> +				IRQF_ONESHOT, "hisi_thermal", data);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to request alarm irq: %d\n", ret);
> +			return ret;
> +		}
>  	}
>  
>  	hisi_thermal_toggle_sensor(&data->sensor, true);
> @@ -398,7 +439,8 @@ static int hisi_thermal_remove(struct platform_device *pdev)
>  	struct hisi_thermal_sensor *sensor = &data->sensor;
>  
>  	hisi_thermal_toggle_sensor(sensor, false);
> -	hi6220_thermal_disable_sensor(data);
> +
> +	data->disable_sensor(data);
>  
>  	return 0;
>  }
> @@ -408,7 +450,7 @@ static int hisi_thermal_suspend(struct device *dev)
>  {
>  	struct hisi_thermal_data *data = dev_get_drvdata(dev);
>  
> -	hi6220_thermal_disable_sensor(data);
> +	data->disable_sensor(data);
>  
>  	return 0;
>  }
> @@ -417,7 +459,7 @@ static int hisi_thermal_resume(struct device *dev)
>  {
>  	struct hisi_thermal_data *data = dev_get_drvdata(dev);
>  
> -	return hi6220_thermal_enable_sensor(data);
> +	return data->enable_sensor(data);
>  }
>  #endif
>  
> -- 
> 2.7.4
> 

Re: [PATCH V2 3/4] thermal/drivers/hisi: Prepare to add support for other hisi platforms

[Daniel Lezcano]

On 20/10/2017 22:58, Eduardo Valentin wrote:
> On Fri, Oct 20, 2017 at 05:11:06PM +0200, Daniel Lezcano wrote:
>> From: Kevin Wangtao <kevin.wangtao@linaro.org>
>>
>> For platform compatibility, add the tsensor ops to a thermal data
>> structure. Each platform has its own probe function to register proper
>> tsensor ops function to the pointer, platform related resource request
>> are also implemented in the platform probe function.
>>
>> Signed-off-by: Kevin Wangtao <kevin.wangtao@linaro.org>
>> Tested-by: Daniel Lezcano <daniel.lezcano@linaro.org> # hikey6220
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> This patch adds this issue to hisi driver (sparse)
> 
> drivers/thermal/hisi_thermal.c:398:24: warning: incorrect type in assignment (different modifiers)
> drivers/thermal/hisi_thermal.c:398:24:    expected int ( *platform_probe )( ... )
> drivers/thermal/hisi_thermal.c:398:24:    got void const *
> 
> essentially you are casting a const into a non const.
> 
> Please fix and resend.

I was not able to reproduce the warning. I tried the C=1, C=2 options,
cross compiled or compiled on x86 with COMPILE_TEST, the warning does
not appear.

Are you using make C=1 or something else to run sparse on the kernel
sources ?



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

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

Re: [PATCH V2 3/4] thermal/drivers/hisi: Prepare to add support for other hisi platforms

[Eduardo Valentin]

On Sat, Oct 21, 2017 at 10:14:33AM +0200, Daniel Lezcano wrote:
> On 20/10/2017 22:58, Eduardo Valentin wrote:
> > On Fri, Oct 20, 2017 at 05:11:06PM +0200, Daniel Lezcano wrote:
> >> From: Kevin Wangtao <kevin.wangtao@linaro.org>
> >>
> >> For platform compatibility, add the tsensor ops to a thermal data
> >> structure. Each platform has its own probe function to register proper
> >> tsensor ops function to the pointer, platform related resource request
> >> are also implemented in the platform probe function.
> >>
> >> Signed-off-by: Kevin Wangtao <kevin.wangtao@linaro.org>
> >> Tested-by: Daniel Lezcano <daniel.lezcano@linaro.org> # hikey6220
> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > 
> > This patch adds this issue to hisi driver (sparse)
> > 
> > drivers/thermal/hisi_thermal.c:398:24: warning: incorrect type in assignment (different modifiers)
> > drivers/thermal/hisi_thermal.c:398:24:    expected int ( *platform_probe )( ... )
> > drivers/thermal/hisi_thermal.c:398:24:    got void const *
> > 
> > essentially you are casting a const into a non const.
> > 
> > Please fix and resend.
> 
> I was not able to reproduce the warning. I tried the C=1, C=2 options,
> cross compiled or compiled on x86 with COMPILE_TEST, the warning does
> not appear.
> 
> Are you using make C=1 or something else to run sparse on the kernel
> sources ?


Yes, this is a make C=1.

The warning is in this code that you add in this patch:
+       platform_probe = of_device_get_match_data(dev);
+       if (!platform_probe) {
+               dev_err(dev, "failed to get probe func\n");
+               return -EINVAL;
        }

platform_probe should be const, because of_device_get_match_data() returns a const:

$ grep -A 10 of_device_get_match_data drivers/of/device.c 
const void *of_device_get_match_data(const struct device *dev)
{
	const struct of_device_id *match;

	match = of_match_device(dev->driver->of_match_table, dev);
	if (!match)
		return NULL;

	return match->data;
}
EXPORT_SYMBOL(of_device_get_match_data);


which I agree, you should use a const to receive the of_device_get_match_data().

> 
> 
> 
> -- 
>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
> 

Re: [PATCH V2 3/4] thermal/drivers/hisi: Prepare to add support for other hisi platforms

[Eduardo Valentin]

Heym

On Sat, Oct 21, 2017 at 08:33:07PM +0200, Daniel Lezcano wrote:
> What sparse version are you using ?


Does it really matter? You are still assigning a const pointer to a non-const pointer.

BR,

Re: [PATCH V2 3/4] thermal/drivers/hisi: Prepare to add support for other hisi platforms

[Daniel Lezcano]

On 21/10/2017 21:08, Eduardo Valentin wrote:
> Heym
> 
> On Sat, Oct 21, 2017 at 08:33:07PM +0200, Daniel Lezcano wrote:
>> What sparse version are you using ?
> 
> 
> Does it really matter? You are still assigning a const pointer to a non-const pointer.

I would like to reproduce it in order to send a correct fix. I'm not
sure about the const function pointer.


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

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

Re: [PATCH V2 3/4] thermal/drivers/hisi: Prepare to add support for other hisi platforms

[Daniel Lezcano]

On 21/10/2017 21:08, Eduardo Valentin wrote:
> Heym
> 
> On Sat, Oct 21, 2017 at 08:33:07PM +0200, Daniel Lezcano wrote:
>> What sparse version are you using ?
> 
> 
> Does it really matter? You are still assigning a const pointer to a non-const pointer.

Finally find the installed sparse version I have is a dev version.
Moving back to the distro version showed the warning.


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

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