[PATCH v1 0/4] Add Mediatek thermal driver for mt2712

[PATCH v1 0/4] Add Mediatek thermal driver for mt2712

[louis.yu]

This series support for mt2712 chip to mtk_thermal.c, and integrate
mt8173, mt2701, and mt2712 on the same mediatek thermal driver.
mt2712 has only one bank and four sensors, and extends calibration
data for mt2712.
Unpaired clock error handle flow is also fixed in this series.

Louis Yu (4):
  dt-bindings: thermal: Add binding document for Mediatek thermal
    controller
  thermal: mediatek: add Mediatek thermal driver for mt2712
  thermal: mediatek: extend calibration data for mt2712 chip
  thermal: mediatek: minor mtk_thermal.c cleanups

 .../bindings/thermal/mediatek-thermal.txt          |  1 +
 drivers/thermal/mtk_thermal.c                      | 90 +++++++++++++++++++---
 2 files changed, 80 insertions(+), 11 deletions(-)

--
1.9.1

[PATCH v1 1/4] dt-bindings: thermal: Add binding document for Mediatek thermal controller

[louis.yu]

From: Louis Yu <louis.yu@mediatek.com>

This adds the device tree binding documentation for the mediatek thermal
controller found on Mediatek MT2712.

Signed-off-by: Louis Yu <louis.yu@mediatek.com>
---
 Documentation/devicetree/bindings/thermal/mediatek-thermal.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt b/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt
index e2f494d..0d73ea5 100644
--- a/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt
@@ -11,6 +11,7 @@ Required properties:
 - compatible:
   - "mediatek,mt8173-thermal" : For MT8173 family of SoCs
   - "mediatek,mt2701-thermal" : For MT2701 family of SoCs
+  - "mediatek,mt2712-thermal" : For MT2712 family of SoCs
 - reg: Address range of the thermal controller
 - interrupts: IRQ for the thermal controller
 - clocks, clock-names: Clocks needed for the thermal controller. required
-- 
1.9.1

[PATCH v1 2/4] thermal: mediatek: add Mediatek thermal driver for mt2712

[louis.yu]

From: Louis Yu <louis.yu@mediatek.com>

This patch adds support for mt2712 chip to mtk_thermal,
and integrate mt2712 into the same mediatek thermal driver.
MT2712 has only 1 bank and 4 sensors.

Signed-off-by: Louis Yu <louis.yu@mediatek.com>
---
 drivers/thermal/mtk_thermal.c | 70 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 66 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index 7737f14..e35d28d 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -3,6 +3,7 @@
  * Author: Hanyi Wu <hanyi.wu@mediatek.com>
  *         Sascha Hauer <s.hauer@pengutronix.de>
  *         Dawei Chien <dawei.chien@mediatek.com>
+ *         Louis Yu <louis.yu@mediatek.com>
  *
  * 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
@@ -111,9 +112,10 @@
 
 /*
  * Layout of the fuses providing the calibration data
- * These macros could be used for both MT8173 and MT2701.
+ * These macros could be used for both MT8173, MT2701, and MT2712.
  * MT8173 has five sensors and need five VTS calibration data,
- * and MT2701 has three sensors and need three VTS calibration data.
+ * and MT2701 has three sensors and need three VTS calibration data,
+ * and MT2712 has four sensors and need four VTS calibration data.
  */
 #define MT8173_CALIB_BUF0_VALID		BIT(0)
 #define MT8173_CALIB_BUF1_ADC_GE(x)	(((x) >> 22) & 0x3ff)
@@ -136,11 +138,26 @@
 /* The total number of temperature sensors in the MT2701 */
 #define MT2701_NUM_SENSORS	3
 
-#define THERMAL_NAME    "mtk-thermal"
-
 /* The number of sensing points per bank */
 #define MT2701_NUM_SENSORS_PER_ZONE	3
 
+/* MT2712 thermal sensors */
+#define MT2712_TS1	0
+#define MT2712_TS2	1
+#define MT2712_TS3	2
+#define MT2712_TS4	3
+
+/* AUXADC channel 11 is used for the temperature sensors */
+#define MT2712_TEMP_AUXADC_CHANNEL	11
+
+/* The total number of temperature sensors in the MT2712 */
+#define MT2712_NUM_SENSORS	4
+
+/* The number of sensing points per bank */
+#define MT2712_NUM_SENSORS_PER_ZONE	4
+
+#define THERMAL_NAME    "mtk-thermal"
+
 struct mtk_thermal;
 
 struct thermal_bank_cfg {
@@ -215,6 +232,21 @@ struct mtk_thermal {
 
 static const int mt2701_mux_values[MT2701_NUM_SENSORS] = { 0, 1, 16 };
 
+/* MT2712 thermal sensor data */
+static const int mt2712_bank_data[MT2712_NUM_SENSORS] = {
+	MT2712_TS1, MT2712_TS2, MT2712_TS3, MT2712_TS4
+};
+
+static const int mt2712_msr[MT2712_NUM_SENSORS_PER_ZONE] = {
+	TEMP_MSR0, TEMP_MSR1, TEMP_MSR2, TEMP_MSR3
+};
+
+static const int mt2712_adcpnp[MT2712_NUM_SENSORS_PER_ZONE] = {
+	TEMP_ADCPNP0, TEMP_ADCPNP1, TEMP_ADCPNP2, TEMP_ADCPNP3
+};
+
+static const int mt2712_mux_values[MT2712_NUM_SENSORS] = { 0, 1, 2, 3 };
+
 /**
  * The MT8173 thermal controller has four banks. Each bank can read up to
  * four temperature sensors simultaneously. The MT8173 has a total of 5
@@ -278,6 +310,31 @@ struct mtk_thermal {
 };
 
 /**
+ * The MT2712 thermal controller has one bank, which can read up to
+ * four temperature sensors simultaneously. The MT2712 has a total of 4
+ * temperature sensors.
+ *
+ * The thermal core only gets the maximum temperature of this one bank,
+ * so the bank concept wouldn't be necessary here. However, the SVS (Smart
+ * Voltage Scaling) unit makes its decisions based on the same bank
+ * data.
+ */
+static const struct mtk_thermal_data mt2712_thermal_data = {
+	.auxadc_channel = MT2712_TEMP_AUXADC_CHANNEL,
+	.num_banks = 1,
+	.num_sensors = MT2712_NUM_SENSORS,
+	.bank_data = {
+		{
+			.num_sensors = 4,
+			.sensors = mt2712_bank_data,
+		},
+	},
+	.msr = mt2712_msr,
+	.adcpnp = mt2712_adcpnp,
+	.sensor_mux_values = mt2712_mux_values,
+};
+
+/**
  * raw_to_mcelsius - convert a raw ADC value to mcelsius
  * @mt:		The thermal controller
  * @raw:	raw ADC value
@@ -571,6 +628,10 @@ static int mtk_thermal_get_calibration_data(struct device *dev,
 	{
 		.compatible = "mediatek,mt2701-thermal",
 		.data = (void *)&mt2701_thermal_data,
+	},
+	{
+		.compatible = "mediatek,mt2712-thermal",
+		.data = (void *)&mt2712_thermal_data,
 	}, {
 	},
 };
@@ -705,6 +766,7 @@ static int mtk_thermal_remove(struct platform_device *pdev)
 
 module_platform_driver(mtk_thermal_driver);
 
+MODULE_AUTHOR("Louis Yu <louis.yu@mediatek.com>");
 MODULE_AUTHOR("Dawei Chien <dawei.chien@mediatek.com>");
 MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
 MODULE_AUTHOR("Hanyi Wu <hanyi.wu@mediatek.com>");
-- 
1.9.1

[PATCH v1 3/4] thermal: mediatek: extend calibration data for mt2712 chip

[louis.yu]

From: Louis Yu <louis.yu@mediatek.com>

This patch adds support for mt2712 chip thermal calibration data
and calculation.

Signed-off-by: Louis Yu <louis.yu@mediatek.com>
---
 drivers/thermal/mtk_thermal.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index e35d28d..f2a7155 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -126,6 +126,8 @@
 #define MT8173_CALIB_BUF2_VTS_TSABB(x)	(((x) >> 14) & 0x1ff)
 #define MT8173_CALIB_BUF0_DEGC_CALI(x)	(((x) >> 1) & 0x3f)
 #define MT8173_CALIB_BUF0_O_SLOPE(x)	(((x) >> 26) & 0x3f)
+#define MT8173_CALIB_BUF0_O_SLOPE_SIGN(x)	(((x) >> 7) & 0x1)
+#define MT8173_CALIB_BUF1_ID(x)	(((x) >> 9) & 0x1)
 
 /* MT2701 thermal sensors */
 #define MT2701_TS1	0
@@ -609,7 +611,11 @@ static int mtk_thermal_get_calibration_data(struct device *dev,
 		mt->vts[MT8173_TS4] = MT8173_CALIB_BUF2_VTS_TS4(buf[2]);
 		mt->vts[MT8173_TSABB] = MT8173_CALIB_BUF2_VTS_TSABB(buf[2]);
 		mt->degc_cali = MT8173_CALIB_BUF0_DEGC_CALI(buf[0]);
-		mt->o_slope = MT8173_CALIB_BUF0_O_SLOPE(buf[0]);
+		if (MT8173_CALIB_BUF1_ID(buf[1]) &
+		    MT8173_CALIB_BUF0_O_SLOPE_SIGN(buf[0]))
+			mt->o_slope = -MT8173_CALIB_BUF0_O_SLOPE(buf[0]);
+		else
+			mt->o_slope = MT8173_CALIB_BUF0_O_SLOPE(buf[0]);
 	} else {
 		dev_info(dev, "Device not calibrated, using default calibration values\n");
 	}
-- 
1.9.1

Re: [PATCH v1 3/4] thermal: mediatek: extend calibration data for mt2712 chip

[Matthias Brugger]

On 07/27/2017 07:46 AM, louis.yu@mediatek.com wrote:
> From: Louis Yu <louis.yu@mediatek.com>
> 
> This patch adds support for mt2712 chip thermal calibration data
> and calculation.
> 
> Signed-off-by: Louis Yu <louis.yu@mediatek.com>
> ---
>   drivers/thermal/mtk_thermal.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index e35d28d..f2a7155 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -126,6 +126,8 @@
>   #define MT8173_CALIB_BUF2_VTS_TSABB(x)	(((x) >> 14) & 0x1ff)
>   #define MT8173_CALIB_BUF0_DEGC_CALI(x)	(((x) >> 1) & 0x3f)
>   #define MT8173_CALIB_BUF0_O_SLOPE(x)	(((x) >> 26) & 0x3f)
> +#define MT8173_CALIB_BUF0_O_SLOPE_SIGN(x)	(((x) >> 7) & 0x1)
> +#define MT8173_CALIB_BUF1_ID(x)	(((x) >> 9) & 0x1)

Why do define them as MT8173_* is this calibration computation generic to 
all/some SoCs?

Regards,
Matthias

>   
>   /* MT2701 thermal sensors */
>   #define MT2701_TS1	0
> @@ -609,7 +611,11 @@ static int mtk_thermal_get_calibration_data(struct device *dev,
>   		mt->vts[MT8173_TS4] = MT8173_CALIB_BUF2_VTS_TS4(buf[2]);
>   		mt->vts[MT8173_TSABB] = MT8173_CALIB_BUF2_VTS_TSABB(buf[2]);
>   		mt->degc_cali = MT8173_CALIB_BUF0_DEGC_CALI(buf[0]);
> -		mt->o_slope = MT8173_CALIB_BUF0_O_SLOPE(buf[0]);
> +		if (MT8173_CALIB_BUF1_ID(buf[1]) &
> +		    MT8173_CALIB_BUF0_O_SLOPE_SIGN(buf[0]))
> +			mt->o_slope = -MT8173_CALIB_BUF0_O_SLOPE(buf[0]);
> +		else
> +			mt->o_slope = MT8173_CALIB_BUF0_O_SLOPE(buf[0]);
>   	} else {
>   		dev_info(dev, "Device not calibrated, using default calibration values\n");
>   	}
> 

Re: [PATCH v1 3/4] thermal: mediatek: extend calibration data for mt2712 chip

[Louis Yu]

Hi Matthias,

On Thu, 2017-07-27 at 17:46 +0200, Matthias Brugger wrote:
> 
> On 07/27/2017 07:46 AM, louis.yu@mediatek.com wrote:
> > From: Louis Yu <louis.yu@mediatek.com>
> > 
> > This patch adds support for mt2712 chip thermal calibration data
> > and calculation.
> > 
> > Signed-off-by: Louis Yu <louis.yu@mediatek.com>
> > ---
> >   drivers/thermal/mtk_thermal.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> > index e35d28d..f2a7155 100644
> > --- a/drivers/thermal/mtk_thermal.c
> > +++ b/drivers/thermal/mtk_thermal.c
> > @@ -126,6 +126,8 @@
> >   #define MT8173_CALIB_BUF2_VTS_TSABB(x)	(((x) >> 14) & 0x1ff)
> >   #define MT8173_CALIB_BUF0_DEGC_CALI(x)	(((x) >> 1) & 0x3f)
> >   #define MT8173_CALIB_BUF0_O_SLOPE(x)	(((x) >> 26) & 0x3f)
> > +#define MT8173_CALIB_BUF0_O_SLOPE_SIGN(x)	(((x) >> 7) & 0x1)
> > +#define MT8173_CALIB_BUF1_ID(x)	(((x) >> 9) & 0x1)
> 
> Why do define them as MT8173_* is this calibration computation generic to 
> all/some SoCs?
> 
> Regards,
> Matthias
> 

Since this calibration computation is generic to all current
upstreamable Mediatek SoCs including mt8173, mt2701, and mt2712. Hence,
The definition of the added bits in calibration data is common for
mt8173, mt2701 and mt2712, and is compatible with these SoCs.

Regards,
Louis

> >   
> >   /* MT2701 thermal sensors */
> >   #define MT2701_TS1	0
> > @@ -609,7 +611,11 @@ static int mtk_thermal_get_calibration_data(struct device *dev,
> >   		mt->vts[MT8173_TS4] = MT8173_CALIB_BUF2_VTS_TS4(buf[2]);
> >   		mt->vts[MT8173_TSABB] = MT8173_CALIB_BUF2_VTS_TSABB(buf[2]);
> >   		mt->degc_cali = MT8173_CALIB_BUF0_DEGC_CALI(buf[0]);
> > -		mt->o_slope = MT8173_CALIB_BUF0_O_SLOPE(buf[0]);
> > +		if (MT8173_CALIB_BUF1_ID(buf[1]) &
> > +		    MT8173_CALIB_BUF0_O_SLOPE_SIGN(buf[0]))
> > +			mt->o_slope = -MT8173_CALIB_BUF0_O_SLOPE(buf[0]);
> > +		else
> > +			mt->o_slope = MT8173_CALIB_BUF0_O_SLOPE(buf[0]);
> >   	} else {
> >   		dev_info(dev, "Device not calibrated, using default calibration values\n");
> >   	}
> > 

[PATCH v1 4/4] thermal: mediatek: minor mtk_thermal.c cleanups

[louis.yu]

From: Louis Yu <louis.yu@mediatek.com>

Move thermal module reset in the beginning and revise clock error
handle flow to pair the clock control.

Signed-off-by: Louis Yu <louis.yu@mediatek.com>
---
 drivers/thermal/mtk_thermal.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index f2a7155..5dffdb4 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -712,20 +712,20 @@ static int mtk_thermal_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	ret = device_reset(&pdev->dev);
+	if (ret)
+		return ret;
+
 	ret = clk_prepare_enable(mt->clk_auxadc);
 	if (ret) {
 		dev_err(&pdev->dev, "Can't enable auxadc clk: %d\n", ret);
-		return ret;
-	}
-
-	ret = device_reset(&pdev->dev);
-	if (ret)
 		goto err_disable_clk_auxadc;
+	}
 
 	ret = clk_prepare_enable(mt->clk_peri_therm);
 	if (ret) {
 		dev_err(&pdev->dev, "Can't enable peri clk: %d\n", ret);
-		goto err_disable_clk_auxadc;
+		goto err_disable_clk_peri_therm;
 	}
 
 	for (i = 0; i < mt->conf->num_banks; i++)
-- 
1.9.1

Re: [PATCH v1 4/4] thermal: mediatek: minor mtk_thermal.c cleanups

[Matthias Brugger]

On 07/27/2017 07:46 AM, louis.yu@mediatek.com wrote:
> From: Louis Yu <louis.yu@mediatek.com>
> 
> Move thermal module reset in the beginning and revise clock error
> handle flow to pair the clock control.
> 
> Signed-off-by: Louis Yu <louis.yu@mediatek.com>
> ---
>   drivers/thermal/mtk_thermal.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index f2a7155..5dffdb4 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -712,20 +712,20 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>   		return -EINVAL;
>   	}
>   
> +	ret = device_reset(&pdev->dev);
> +	if (ret)
> +		return ret;
> +
>   	ret = clk_prepare_enable(mt->clk_auxadc);
>   	if (ret) {
>   		dev_err(&pdev->dev, "Can't enable auxadc clk: %d\n", ret);
> -		return ret;
> -	}
> -
> -	ret = device_reset(&pdev->dev);
> -	if (ret)
>   		goto err_disable_clk_auxadc;
> +	}
>   
>   	ret = clk_prepare_enable(mt->clk_peri_therm);
>   	if (ret) {
>   		dev_err(&pdev->dev, "Can't enable peri clk: %d\n", ret);
> -		goto err_disable_clk_auxadc;
> +		goto err_disable_clk_peri_therm;

If you can't enable the clock, there is no use in disable it in the error path.

Regards,
Matthias

>   	}
>   
>   	for (i = 0; i < mt->conf->num_banks; i++)
> 

Re: [PATCH v1 4/4] thermal: mediatek: minor mtk_thermal.c cleanups

[Louis Yu]

Hi Matthias,

On Thu, 2017-07-27 at 17:49 +0200, Matthias Brugger wrote:
> 
> On 07/27/2017 07:46 AM, louis.yu@mediatek.com wrote:
> > From: Louis Yu <louis.yu@mediatek.com>
> > 
> > Move thermal module reset in the beginning and revise clock error
> > handle flow to pair the clock control.
> > 
> > Signed-off-by: Louis Yu <louis.yu@mediatek.com>
> > ---
> >   drivers/thermal/mtk_thermal.c | 12 ++++++------
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> > index f2a7155..5dffdb4 100644
> > --- a/drivers/thermal/mtk_thermal.c
> > +++ b/drivers/thermal/mtk_thermal.c
> > @@ -712,20 +712,20 @@ static int mtk_thermal_probe(struct platform_device *pdev)
> >   		return -EINVAL;
> >   	}
> >   
> > +	ret = device_reset(&pdev->dev);
> > +	if (ret)
> > +		return ret;
> > +
> >   	ret = clk_prepare_enable(mt->clk_auxadc);
> >   	if (ret) {
> >   		dev_err(&pdev->dev, "Can't enable auxadc clk: %d\n", ret);
> > -		return ret;
> > -	}
> > -
> > -	ret = device_reset(&pdev->dev);
> > -	if (ret)
> >   		goto err_disable_clk_auxadc;
> > +	}
> >   
> >   	ret = clk_prepare_enable(mt->clk_peri_therm);
> >   	if (ret) {
> >   		dev_err(&pdev->dev, "Can't enable peri clk: %d\n", ret);
> > -		goto err_disable_clk_auxadc;
> > +		goto err_disable_clk_peri_therm;
> 
> If you can't enable the clock, there is no use in disable it in the error path.
> 
> Regards,
> Matthias
> 

Thanks for your reminder.This is no use to disable it in the error path.
I would update this in next version as follows.

diff --git a/drivers/thermal/mtk_thermal.c
b/drivers/thermal/mtk_thermal.c
index 5dffdb4..3df0de3 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -719,13 +719,13 @@ static int mtk_thermal_probe(struct
platform_device *pdev)
 	ret = clk_prepare_enable(mt->clk_auxadc);
 	if (ret) {
 		dev_err(&pdev->dev, "Can't enable auxadc clk: %d\n", ret);
-		goto err_disable_clk_auxadc;
+		return ret;
 	}
 
 	ret = clk_prepare_enable(mt->clk_peri_therm);
 	if (ret) {
 		dev_err(&pdev->dev, "Can't enable peri clk: %d\n", ret);
-		goto err_disable_clk_peri_therm;
+		goto err_disable_clk_auxadc;
 	}
 
 	for (i = 0; i < mt->conf->num_banks; i++)


Regards
Louis

> >   	}
> >   
> >   	for (i = 0; i < mt->conf->num_banks; i++)
> >