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

[PATCH v2 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 and the existing chips.

Changes since v1:
- fix wrong clock error handle flow from Matthias' comment.
- make clear comment for the extended calibration data.

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                      | 88 +++++++++++++++++++---
 2 files changed, 79 insertions(+), 10 deletions(-)

--
1.8.1.1.dirty

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

[Louis Yu]

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.8.1.1.dirty

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

[Louis Yu]

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.8.1.1.dirty

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

[Zhang Rui]

On Tue, 2017-08-01 at 15:28 +0800, Louis Yu wrote:
> 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>

I need the feedback from the previous authors for the changes in all
this patch set.

>   *
>   * 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.
>   */

why bother change these comments again in patch 3/4?

thanks,
rui
>  #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>");

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

[Louis Yu]

Hi Zhang,

On Tue, 2017-08-08 at 21:23 +0800, Zhang Rui wrote:
> On Tue, 2017-08-01 at 15:28 +0800, Louis Yu wrote:
> > 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>
> 
> I need the feedback from the previous authors for the changes in all
> this patch set.

Thanks for looping the previous authors for the feedback.

> >   *
> >   * 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.
> >   */
> 
> why bother change these comments again in patch 3/4?
> 
> thanks,
> rui

The comment in patch 2/4 is to add support for mt2712, and the comment
in patch 3/4 is to make calibration comments more clear and to fix some
grammar error. Hence, I change these comments in separated patches.

Regards,
Louis

> >  #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>");

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

[Louis Yu]

Hi Rui,

On Thu, 2017-08-10 at 11:32 +0800, Louis Yu wrote:
> Hi Zhang,
> 
> On Tue, 2017-08-08 at 21:23 +0800, Zhang Rui wrote:
> > On Tue, 2017-08-01 at 15:28 +0800, Louis Yu wrote:
> > > 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>
> > 
> > I need the feedback from the previous authors for the changes in all
> > this patch set.
> 
> Thanks for looping the previous authors for the feedback.
> 
> > >   *
> > >   * 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.
> > >   */
> > 
> > why bother change these comments again in patch 3/4?
> > 
> > thanks,
> > rui
> 
> The comment in patch 2/4 is to add support for mt2712, and the comment
> in patch 3/4 is to make calibration comments more clear and to fix some
> grammar error. Hence, I change these comments in separated patches.
> 
> Regards,
> Louis

If any part is not clear or any suggestions, please feel free to let me
know. If no further questions, I would keep this version for the review.

Thanks,
Louis

> > >  #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>");
> 

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

[Zhang Rui]

On Tue, 2017-08-08 at 21:23 +0800, Zhang Rui wrote:
> On Tue, 2017-08-01 at 15:28 +0800, Louis Yu wrote:
> > 
> > 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>
> I need the feedback from the previous authors for the changes in all
> this patch set.
> 
Hanyi, Sascha and Dawei,

can you please review this patch and let me know your opinion?

thanks,
rui
> > 
> >   *
> >   * 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.
> >   */
> why bother change these comments again in patch 3/4?
> 
> thanks,
> rui
> > 
> >  #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>");

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

[Dawei Chien]

On Fri, 2017-08-25 at 10:30 +0800, Zhang Rui wrote:
> On Tue, 2017-08-08 at 21:23 +0800, Zhang Rui wrote:
> > On Tue, 2017-08-01 at 15:28 +0800, Louis Yu wrote:
> > > 
> > > 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>
> > I need the feedback from the previous authors for the changes in all
> > this patch set.
> > 
> Hanyi, Sascha and Dawei,
> 
> can you please review this patch and let me know your opinion?
> 
> thanks,
> rui

Hi Rui,
In my opinion, I agree this patch set for supporting MT2712 thermal
sensor.

Reviewed-by: Dawei Chien <dawei.chien@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.
> > >   */
> > why bother change these comments again in patch 3/4?
> > 
> > thanks,
> > rui

Hi Rui,
May we know your opinion if this patch set need merge patch2/3/4

> > > 
> > >  #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>");

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

[Zhang Rui]

Hi, Dawei,


On Fri, 2017-08-25 at 11:10 +0800, Dawei Chien wrote:
> On Fri, 2017-08-25 at 10:30 +0800, Zhang Rui wrote:
> > 
> > On Tue, 2017-08-08 at 21:23 +0800, Zhang Rui wrote:
> > > 
> > > On Tue, 2017-08-01 at 15:28 +0800, Louis Yu wrote:
> > > > 
> > > > 
> > > > 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>
> > > I need the feedback from the previous authors for the changes in
> > > all
> > > this patch set.
> > > 
> > Hanyi, Sascha and Dawei,
> > 
> > can you please review this patch and let me know your opinion?
> > 
> > thanks,
> > rui
> Hi Rui,
> In my opinion, I agree this patch set for supporting MT2712 thermal
> sensor.
> 
> Reviewed-by: Dawei Chien <dawei.chien@mediatek.com>
> 
thanks for reviewing.
> > 
> > > 
> > > > 
> > > > 
> > > >   *
> > > >   * 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.
> > > >   */
> > > why bother change these comments again in patch 3/4?
> > > 
> > > thanks,
> > > rui
> Hi Rui,
> May we know your opinion if this patch set need merge patch2/3/4
> 
what do you mean? If you're okay with this, I will merge the full patch
set.

thanks,
rui
> > 
> > > 
> > > > 
> > > > 
> > > >  #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>");
> 

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

[Dawei Chien]

On Fri, 2017-08-25 at 11:23 +0800, Zhang Rui wrote:
> Hi, Dawei,
> 
> 
> On Fri, 2017-08-25 at 11:10 +0800, Dawei Chien wrote:
> > On Fri, 2017-08-25 at 10:30 +0800, Zhang Rui wrote:
> > > 
> > > On Tue, 2017-08-08 at 21:23 +0800, Zhang Rui wrote:
> > > > 
> > > > On Tue, 2017-08-01 at 15:28 +0800, Louis Yu wrote:
> > > > > 
> > > > > 
> > > > > 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>
> > > > I need the feedback from the previous authors for the changes in
> > > > all
> > > > this patch set.
> > > > 
> > > Hanyi, Sascha and Dawei,
> > > 
> > > can you please review this patch and let me know your opinion?
> > > 
> > > thanks,
> > > rui
> > Hi Rui,
> > In my opinion, I agree this patch set for supporting MT2712 thermal
> > sensor.
> > 
> > Reviewed-by: Dawei Chien <dawei.chien@mediatek.com>
> > 
> thanks for reviewing.
> > > 
> > > > 
> > > > > 
> > > > > 
> > > > >   *
> > > > >   * 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.
> > > > >   */
> > > > why bother change these comments again in patch 3/4?
> > > > 
> > > > thanks,
> > > > rui
> > Hi Rui,
> > May we know your opinion if this patch set need merge patch2/3/4
> > 
> what do you mean? If you're okay with this, I will merge the full patch
> set.
> 
> thanks,
> rui

Hi Rui,
It's okay to me. Since we saw your comment "why bother change these
comments again in patch 3/4?", so I think you might suggest to merge
patch2/patch3/patch4 to one patch.

BR,
Dawei

> > > 
> > > > 
> > > > > 
> > > > > 
> > > > >  #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>");
> > 

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

[Zhang Rui]

On Fri, 2017-08-25 at 11:39 +0800, Dawei Chien wrote:
> On Fri, 2017-08-25 at 11:23 +0800, Zhang Rui wrote:
> > 
> > Hi, Dawei,
> > 
> > 
> > On Fri, 2017-08-25 at 11:10 +0800, Dawei Chien wrote:
> > > 
> > > On Fri, 2017-08-25 at 10:30 +0800, Zhang Rui wrote:
> > > > 
> > > > 
> > > > On Tue, 2017-08-08 at 21:23 +0800, Zhang Rui wrote:
> > > > > 
> > > > > 
> > > > > On Tue, 2017-08-01 at 15:28 +0800, Louis Yu wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 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>
> > > > > I need the feedback from the previous authors for the changes
> > > > > in
> > > > > all
> > > > > this patch set.
> > > > > 
> > > > Hanyi, Sascha and Dawei,
> > > > 
> > > > can you please review this patch and let me know your opinion?
> > > > 
> > > > thanks,
> > > > rui
> > > Hi Rui,
> > > In my opinion, I agree this patch set for supporting MT2712
> > > thermal
> > > sensor.
> > > 
> > > Reviewed-by: Dawei Chien <dawei.chien@mediatek.com>
> > > 
> > thanks for reviewing.
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > >   *
> > > > > >   * 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.
> > > > > >   */
> > > > > why bother change these comments again in patch 3/4?
> > > > > 
> > > > > thanks,
> > > > > rui
> > > Hi Rui,
> > > May we know your opinion if this patch set need merge patch2/3/4
> > > 
> > what do you mean? If you're okay with this, I will merge the full
> > patch
> > set.
> > 
> > thanks,
> > rui
> Hi Rui,
> It's okay to me. Since we saw your comment "why bother change these
> comments again in patch 3/4?", so I think you might suggest to merge
> patch2/patch3/patch4 to one patch.
> 
I will take patch 2, 3 and 4 as separate patches.
BTW, is it okay for me to add your Reviewed-by for all the patch
series?

thanks,
rui

> BR,
> Dawei
> 
> > 
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > >  #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>");
> 

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

[Matthias Brugger]

On 08/31/2017 04:10 PM, Zhang Rui wrote:
> On Fri, 2017-08-25 at 11:39 +0800, Dawei Chien wrote:
>> On Fri, 2017-08-25 at 11:23 +0800, Zhang Rui wrote:
>>>
>>> Hi, Dawei,
>>>
>>>
>>> On Fri, 2017-08-25 at 11:10 +0800, Dawei Chien wrote:
>>>>
>>>> On Fri, 2017-08-25 at 10:30 +0800, Zhang Rui wrote:
>>>>>
>>>>>
>>>>> On Tue, 2017-08-08 at 21:23 +0800, Zhang Rui wrote:
>>>>>>
>>>>>>
>>>>>> On Tue, 2017-08-01 at 15:28 +0800, Louis Yu wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 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>
>>>>>> I need the feedback from the previous authors for the changes
>>>>>> in
>>>>>> all
>>>>>> this patch set.
>>>>>>
>>>>> Hanyi, Sascha and Dawei,
>>>>>
>>>>> can you please review this patch and let me know your opinion?
>>>>>
>>>>> thanks,
>>>>> rui
>>>> Hi Rui,
>>>> In my opinion, I agree this patch set for supporting MT2712
>>>> thermal
>>>> sensor.
>>>>
>>>> Reviewed-by: Dawei Chien <dawei.chien@mediatek.com>
>>>>
>>> thanks for reviewing.
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>    *
>>>>>>>    * 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.
>>>>>>>    */
>>>>>> why bother change these comments again in patch 3/4?
>>>>>>
>>>>>> thanks,
>>>>>> rui
>>>> Hi Rui,
>>>> May we know your opinion if this patch set need merge patch2/3/4
>>>>
>>> what do you mean? If you're okay with this, I will merge the full
>>> patch
>>> set.
>>>
>>> thanks,
>>> rui
>> Hi Rui,
>> It's okay to me. Since we saw your comment "why bother change these
>> comments again in patch 3/4?", so I think you might suggest to merge
>> patch2/patch3/patch4 to one patch.
>>
> I will take patch 2, 3 and 4 as separate patches.
> BTW, is it okay for me to add your Reviewed-by for all the patch
> series?
> 
> thanks,
> rui
> 

Hi Rui,

Normally the driver maintainers also take the devicetree bindings documentation 
and the SoC maintainter takes the acutal dts declaration for each SoC.

Please let me know if you prefer, that I take care of the bindings documentation 
as well.

Best regards,
Matthias

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

[Dawei Chien]

On Thu, 2017-08-31 at 21:10 +0800, Zhang Rui wrote:
> On Fri, 2017-08-25 at 11:39 +0800, Dawei Chien wrote:
> > On Fri, 2017-08-25 at 11:23 +0800, Zhang Rui wrote:
> > > 
> > > Hi, Dawei,
> > > 
> > > 
> > > On Fri, 2017-08-25 at 11:10 +0800, Dawei Chien wrote:
> > > > 
> > > > On Fri, 2017-08-25 at 10:30 +0800, Zhang Rui wrote:
> > > > > 
> > > > > 
> > > > > On Tue, 2017-08-08 at 21:23 +0800, Zhang Rui wrote:
> > > > > > 
> > > > > > 
> > > > > > On Tue, 2017-08-01 at 15:28 +0800, Louis Yu wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 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>
> > > > > > I need the feedback from the previous authors for the changes
> > > > > > in
> > > > > > all
> > > > > > this patch set.
> > > > > > 
> > > > > Hanyi, Sascha and Dawei,
> > > > > 
> > > > > can you please review this patch and let me know your opinion?
> > > > > 
> > > > > thanks,
> > > > > rui
> > > > Hi Rui,
> > > > In my opinion, I agree this patch set for supporting MT2712
> > > > thermal
> > > > sensor.
> > > > 
> > > > Reviewed-by: Dawei Chien <dawei.chien@mediatek.com>
> > > > 
> > > thanks for reviewing.
> > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > >   *
> > > > > > >   * 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.
> > > > > > >   */
> > > > > > why bother change these comments again in patch 3/4?
> > > > > > 
> > > > > > thanks,
> > > > > > rui
> > > > Hi Rui,
> > > > May we know your opinion if this patch set need merge patch2/3/4
> > > > 
> > > what do you mean? If you're okay with this, I will merge the full
> > > patch
> > > set.
> > > 
> > > thanks,
> > > rui
> > Hi Rui,
> > It's okay to me. Since we saw your comment "why bother change these
> > comments again in patch 3/4?", so I think you might suggest to merge
> > patch2/patch3/patch4 to one patch.
> > 
> I will take patch 2, 3 and 4 as separate patches.
> BTW, is it okay for me to add your Reviewed-by for all the patch
> series?
> 
> thanks,
> rui
> 
Hi Rui,
Yes, please add my Reviewed-by for all the patch series, thank you.

BR,
Dawei
> > 
> > > 
> > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > >  #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>");
> > 

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

[Louis Yu]

This patch adds support for mt2712 chip thermal calibration data
and calculation, and is compatible with the existing chips.

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

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index e35d28d..b609589 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -112,10 +112,10 @@
 
 /*
  * Layout of the fuses providing the calibration data
- * 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 MT2712 has four sensors and need four VTS calibration data.
+ * These macros could be used for MT8173, MT2701, and MT2712.
+ * MT8173 has 5 sensors and needs 5 VTS calibration data.
+ * MT2701 has 3 sensors and needs 3 VTS calibration data.
+ * MT2712 has 4 sensors and needs 4 VTS calibration data.
  */
 #define MT8173_CALIB_BUF0_VALID		BIT(0)
 #define MT8173_CALIB_BUF1_ADC_GE(x)	(((x) >> 22) & 0x3ff)
@@ -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.8.1.1.dirty

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

[Louis Yu]

Move independent thermal module reset in the beginning.

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

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index b609589..1e61c09 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -712,16 +712,16 @@ 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);
-- 
1.8.1.1.dirty