[PATCH 0/3] thermal: mediatek: Add mt8365 support

[PATCH 0/3] thermal: mediatek: Add mt8365 support

[Markus Schneider-Pargmann]

Hi,

this series adds a compatible to the bindings and a specific part for
the thermal driver.

Thanks for any feedback or comments.

Best,
Markus


Fabien Parent (1):
  thermal: mediatek: add MT8365 thermal driver support

Markus Schneider-Pargmann (2):
  dt-bindings: thermal: mediatek: Add mt8365
  thermal: mediatek: Fix apmixed error message

 .../bindings/thermal/mediatek-thermal.txt     |  1 +
 drivers/thermal/mtk_thermal.c                 | 93 +++++++++++++++++--
 2 files changed, 87 insertions(+), 7 deletions(-)

-- 
2.33.0

[PATCH 1/3] dt-bindings: thermal: mediatek: Add mt8365

[Markus Schneider-Pargmann]

This unit is present on the mt8365 SoC as well. But there is a
difference in the usage of an apmixed register.

This patch adds a distinct mt8365 to the list of compatibles.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.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 5c7e7bdd029a..ba4ebffeade4 100644
--- a/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt
@@ -14,6 +14,7 @@ Required properties:
   - "mediatek,mt2712-thermal" : For MT2712 family of SoCs
   - "mediatek,mt7622-thermal" : For MT7622 SoC
   - "mediatek,mt8183-thermal" : For MT8183 family of SoCs
+  - "mediatek,mt8365-thermal" : For MT8365 family of SoCs
   - "mediatek,mt8516-thermal", "mediatek,mt2701-thermal : For MT8516 family of SoCs
 - reg: Address range of the thermal controller
 - interrupts: IRQ for the thermal controller
-- 
2.33.0

[PATCH 2/3] thermal: mediatek: Fix apmixed error message

[Markus Schneider-Pargmann]

Seems to be a copy and paste mistake, this is the apmixed address, not
auxadc.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
Fixes: a92db1c8089e (thermal: Add Mediatek thermal controller support)
---
 drivers/thermal/mtk_thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index ede94eadddda..93ee043d70da 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -1050,7 +1050,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
 	of_node_put(apmixedsys);
 
 	if (apmixed_phys_base == OF_BAD_ADDR) {
-		dev_err(&pdev->dev, "Can't get auxadc phys address\n");
+		dev_err(&pdev->dev, "Can't get apmixed phys address\n");
 		return -EINVAL;
 	}
 
-- 
2.33.0

[PATCH 3/3] thermal: mediatek: add MT8365 thermal driver support

[Markus Schneider-Pargmann]

From: Fabien Parent <fparent@baylibre.com>

mt8365 is similar to the other SoCs supported by the driver. It has only
one bank and 3 sensors that can be multiplexed.

Additionally the buffer has to be enabled and connected to AUXADC
similar to the V2 version but at a different register offset. That's why
I added three new configuration values to define the register, mask and
bits to be set to be able to use it for both V2 and mt8365.

Signed-off-by: Fabien Parent <fparent@baylibre.com>
[Added apmixed control register logic]
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/thermal/mtk_thermal.c | 91 ++++++++++++++++++++++++++++++++---
 1 file changed, 85 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index 93ee043d70da..7a75ae8231f2 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -31,6 +31,7 @@
 #define AUXADC_CON2_V		0x010
 #define AUXADC_DATA(channel)	(0x14 + (channel) * 4)
 
+#define APMIXED_SYS_TS_CON0	0x600
 #define APMIXED_SYS_TS_CON1	0x604
 
 /* Thermal Controller Registers */
@@ -245,6 +246,17 @@ enum mtk_thermal_version {
 /* The calibration coefficient of sensor  */
 #define MT8183_CALIBRATION	153
 
+/* MT8365 */
+#define MT8365_TEMP_AUXADC_CHANNEL 11
+#define MT8365_CALIBRATION 164
+#define MT8365_NUM_CONTROLLER 1
+#define MT8365_NUM_BANKS 1
+#define MT8365_NUM_SENSORS 3
+#define MT8365_NUM_SENSORS_PER_ZONE 3
+#define MT8365_TS1 0
+#define MT8365_TS2 1
+#define MT8365_TS3 2
+
 struct mtk_thermal;
 
 struct thermal_bank_cfg {
@@ -271,6 +283,9 @@ struct mtk_thermal_data {
 	bool need_switch_bank;
 	struct thermal_bank_cfg bank_data[MAX_NUM_ZONES];
 	enum mtk_thermal_version version;
+	u32 apmixed_buffer_ctl_reg;
+	u32 apmixed_buffer_ctl_mask;
+	u32 apmixed_buffer_ctl_set;
 };
 
 struct mtk_thermal {
@@ -386,6 +401,24 @@ static const int mt7622_mux_values[MT7622_NUM_SENSORS] = { 0, };
 static const int mt7622_vts_index[MT7622_NUM_SENSORS] = { VTS1 };
 static const int mt7622_tc_offset[MT7622_NUM_CONTROLLER] = { 0x0, };
 
+/* MT8365 thermal sensor data */
+static const int mt8365_bank_data[MT8365_NUM_SENSORS] = {
+	MT8365_TS1, MT8365_TS2, MT8365_TS3
+};
+
+static const int mt8365_msr[MT8365_NUM_SENSORS_PER_ZONE] = {
+	TEMP_MSR0, TEMP_MSR1, TEMP_MSR2
+};
+
+static const int mt8365_adcpnp[MT8365_NUM_SENSORS_PER_ZONE] = {
+	TEMP_ADCPNP0, TEMP_ADCPNP1, TEMP_ADCPNP2
+};
+
+static const int mt8365_mux_values[MT8365_NUM_SENSORS] = { 0, 1, 2 };
+static const int mt8365_tc_offset[MT8365_NUM_CONTROLLER] = { 0 };
+
+static const int mt8365_vts_index[MT8365_NUM_SENSORS] = { VTS1, VTS2, VTS3 };
+
 /*
  * The MT8173 thermal controller has four banks. Each bank can read up to
  * four temperature sensors simultaneously. The MT8173 has a total of 5
@@ -460,6 +493,40 @@ static const struct mtk_thermal_data mt2701_thermal_data = {
 	.version = MTK_THERMAL_V1,
 };
 
+/*
+ * The MT8365 thermal controller has one bank, which can read up to
+ * four temperature sensors simultaneously. The MT8365 has a total of 3
+ * 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 mt8365_thermal_data = {
+	.auxadc_channel = MT8365_TEMP_AUXADC_CHANNEL,
+	.num_banks = MT8365_NUM_BANKS,
+	.num_sensors = MT8365_NUM_SENSORS,
+	.vts_index = mt8365_vts_index,
+	.cali_val = MT8365_CALIBRATION,
+	.num_controller = MT8365_NUM_CONTROLLER,
+	.controller_offset = mt8365_tc_offset,
+	.need_switch_bank = false,
+	.bank_data = {
+		{
+			.num_sensors = MT8365_NUM_SENSORS,
+			.sensors = mt8365_bank_data
+		},
+	},
+	.msr = mt8365_msr,
+	.adcpnp = mt8365_adcpnp,
+	.sensor_mux_values = mt8365_mux_values,
+	.version = MTK_THERMAL_V1,
+	.apmixed_buffer_ctl_reg = APMIXED_SYS_TS_CON0,
+	.apmixed_buffer_ctl_mask = ~(u32)GENMASK(29, 28),
+	.apmixed_buffer_ctl_set = 0,
+};
+
 /*
  * The MT2712 thermal controller has one bank, which can read up to
  * four temperature sensors simultaneously. The MT2712 has a total of 4
@@ -514,6 +581,9 @@ static const struct mtk_thermal_data mt7622_thermal_data = {
 	.adcpnp = mt7622_adcpnp,
 	.sensor_mux_values = mt7622_mux_values,
 	.version = MTK_THERMAL_V2,
+	.apmixed_buffer_ctl_reg = APMIXED_SYS_TS_CON1,
+	.apmixed_buffer_ctl_mask = ~0x37,
+	.apmixed_buffer_ctl_set = 0x1,
 };
 
 /*
@@ -958,19 +1028,27 @@ static const struct of_device_id mtk_thermal_of_match[] = {
 	{
 		.compatible = "mediatek,mt8183-thermal",
 		.data = (void *)&mt8183_thermal_data,
+	},
+	{
+		.compatible = "mediatek,mt8365-thermal",
+		.data = (void *)&mt8365_thermal_data,
 	}, {
 	},
 };
 MODULE_DEVICE_TABLE(of, mtk_thermal_of_match);
 
-static void mtk_thermal_turn_on_buffer(void __iomem *apmixed_base)
+static void mtk_thermal_turn_on_buffer(struct mtk_thermal *mt,
+				       void __iomem *apmixed_base)
 {
 	int tmp;
 
-	tmp = readl(apmixed_base + APMIXED_SYS_TS_CON1);
-	tmp &= ~(0x37);
-	tmp |= 0x1;
-	writel(tmp, apmixed_base + APMIXED_SYS_TS_CON1);
+	if (!mt->conf->apmixed_buffer_ctl_reg)
+		return;
+
+	tmp = readl(apmixed_base + mt->conf->apmixed_buffer_ctl_reg);
+	tmp &= mt->conf->apmixed_buffer_ctl_mask;
+	tmp |= mt->conf->apmixed_buffer_ctl_set;
+	writel(tmp, apmixed_base + mt->conf->apmixed_buffer_ctl_reg);
 	udelay(200);
 }
 
@@ -1070,8 +1148,9 @@ static int mtk_thermal_probe(struct platform_device *pdev)
 		goto err_disable_clk_auxadc;
 	}
 
+	mtk_thermal_turn_on_buffer(mt, apmixed_base);
+
 	if (mt->conf->version == MTK_THERMAL_V2) {
-		mtk_thermal_turn_on_buffer(apmixed_base);
 		mtk_thermal_release_periodic_ts(mt, auxadc_base);
 	}
 
-- 
2.33.0

Re: [PATCH 3/3] thermal: mediatek: add MT8365 thermal driver support

[Daniel Lezcano]

On 14/10/2021 15:56, Markus Schneider-Pargmann wrote:
> From: Fabien Parent <fparent@baylibre.com>
> 
> mt8365 is similar to the other SoCs supported by the driver. It has only
> one bank and 3 sensors that can be multiplexed.
> 
> Additionally the buffer has to be enabled and connected to AUXADC
> similar to the V2 version but at a different register offset. That's why
> I added three new configuration values to define the register, mask and
> bits to be set to be able to use it for both V2 and mt8365.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> [Added apmixed control register logic]
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  drivers/thermal/mtk_thermal.c | 91 ++++++++++++++++++++++++++++++++---
>  1 file changed, 85 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index 93ee043d70da..7a75ae8231f2 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -31,6 +31,7 @@
>  #define AUXADC_CON2_V		0x010
>  #define AUXADC_DATA(channel)	(0x14 + (channel) * 4)
>  
> +#define APMIXED_SYS_TS_CON0	0x600
>  #define APMIXED_SYS_TS_CON1	0x604
>  
>  /* Thermal Controller Registers */
> @@ -245,6 +246,17 @@ enum mtk_thermal_version {
>  /* The calibration coefficient of sensor  */
>  #define MT8183_CALIBRATION	153
>  
> +/* MT8365 */
> +#define MT8365_TEMP_AUXADC_CHANNEL 11
> +#define MT8365_CALIBRATION 164
> +#define MT8365_NUM_CONTROLLER 1
> +#define MT8365_NUM_BANKS 1
> +#define MT8365_NUM_SENSORS 3
> +#define MT8365_NUM_SENSORS_PER_ZONE 3

You can get rid of these macros by using ARRAY_SIZE.

eg.

static const int mt8365_bank_data[] = {
	MT8365_TS1,
	MT8365_TS2,
	MT8365_TS3
};

...

	.num_sensors = ARRAY_SIZE(mt8365_bank_data);
...


> +#define MT8365_TS1 0
> +#define MT8365_TS2 1
> +#define MT8365_TS3 2
> +
>  struct mtk_thermal;
>  
>  struct thermal_bank_cfg {
> @@ -271,6 +283,9 @@ struct mtk_thermal_data {
>  	bool need_switch_bank;
>  	struct thermal_bank_cfg bank_data[MAX_NUM_ZONES];
>  	enum mtk_thermal_version version;
> +	u32 apmixed_buffer_ctl_reg;
> +	u32 apmixed_buffer_ctl_mask;
> +	u32 apmixed_buffer_ctl_set;
>  };
>  
>  struct mtk_thermal {
> @@ -386,6 +401,24 @@ static const int mt7622_mux_values[MT7622_NUM_SENSORS] = { 0, };
>  static const int mt7622_vts_index[MT7622_NUM_SENSORS] = { VTS1 };
>  static const int mt7622_tc_offset[MT7622_NUM_CONTROLLER] = { 0x0, };
>  
> +/* MT8365 thermal sensor data */
> +static const int mt8365_bank_data[MT8365_NUM_SENSORS] = {
> +	MT8365_TS1, MT8365_TS2, MT8365_TS3
> +};
> +
> +static const int mt8365_msr[MT8365_NUM_SENSORS_PER_ZONE] = {
> +	TEMP_MSR0, TEMP_MSR1, TEMP_MSR2
> +};
> +
> +static const int mt8365_adcpnp[MT8365_NUM_SENSORS_PER_ZONE] = {
> +	TEMP_ADCPNP0, TEMP_ADCPNP1, TEMP_ADCPNP2
> +};
> +
> +static const int mt8365_mux_values[MT8365_NUM_SENSORS] = { 0, 1, 2 };
> +static const int mt8365_tc_offset[MT8365_NUM_CONTROLLER] = { 0 };
> +
> +static const int mt8365_vts_index[MT8365_NUM_SENSORS] = { VTS1, VTS2, VTS3 };
> +
>  /*
>   * The MT8173 thermal controller has four banks. Each bank can read up to
>   * four temperature sensors simultaneously. The MT8173 has a total of 5
> @@ -460,6 +493,40 @@ static const struct mtk_thermal_data mt2701_thermal_data = {
>  	.version = MTK_THERMAL_V1,
>  };
>  
> +/*
> + * The MT8365 thermal controller has one bank, which can read up to
> + * four temperature sensors simultaneously. The MT8365 has a total of 3
> + * 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 mt8365_thermal_data = {
> +	.auxadc_channel = MT8365_TEMP_AUXADC_CHANNEL,
> +	.num_banks = MT8365_NUM_BANKS,
> +	.num_sensors = MT8365_NUM_SENSORS,
> +	.vts_index = mt8365_vts_index,
> +	.cali_val = MT8365_CALIBRATION,
> +	.num_controller = MT8365_NUM_CONTROLLER,
> +	.controller_offset = mt8365_tc_offset,
> +	.need_switch_bank = false,
> +	.bank_data = {
> +		{
> +			.num_sensors = MT8365_NUM_SENSORS,
> +			.sensors = mt8365_bank_data
> +		},
> +	},
> +	.msr = mt8365_msr,
> +	.adcpnp = mt8365_adcpnp,
> +	.sensor_mux_values = mt8365_mux_values,
> +	.version = MTK_THERMAL_V1,
> +	.apmixed_buffer_ctl_reg = APMIXED_SYS_TS_CON0,
> +	.apmixed_buffer_ctl_mask = ~(u32)GENMASK(29, 28),
> +	.apmixed_buffer_ctl_set = 0,
> +};
> +
>  /*
>   * The MT2712 thermal controller has one bank, which can read up to
>   * four temperature sensors simultaneously. The MT2712 has a total of 4
> @@ -514,6 +581,9 @@ static const struct mtk_thermal_data mt7622_thermal_data = {
>  	.adcpnp = mt7622_adcpnp,
>  	.sensor_mux_values = mt7622_mux_values,
>  	.version = MTK_THERMAL_V2,
> +	.apmixed_buffer_ctl_reg = APMIXED_SYS_TS_CON1,
> +	.apmixed_buffer_ctl_mask = ~0x37,
> +	.apmixed_buffer_ctl_set = 0x1,

Please change those literal into macros

>  };
>  
>  /*
> @@ -958,19 +1028,27 @@ static const struct of_device_id mtk_thermal_of_match[] = {
>  	{
>  		.compatible = "mediatek,mt8183-thermal",
>  		.data = (void *)&mt8183_thermal_data,
> +	},
> +	{
> +		.compatible = "mediatek,mt8365-thermal",
> +		.data = (void *)&mt8365_thermal_data,

Is this cast really needed ?

>  	}, {
>  	},
>  };
>  MODULE_DEVICE_TABLE(of, mtk_thermal_of_match);
>  
> -static void mtk_thermal_turn_on_buffer(void __iomem *apmixed_base)
> +static void mtk_thermal_turn_on_buffer(struct mtk_thermal *mt,
> +				       void __iomem *apmixed_base)
>  {
>  	int tmp;
>  
> -	tmp = readl(apmixed_base + APMIXED_SYS_TS_CON1);
> -	tmp &= ~(0x37);
> -	tmp |= 0x1;
> -	writel(tmp, apmixed_base + APMIXED_SYS_TS_CON1);
> +	if (!mt->conf->apmixed_buffer_ctl_reg)
> +		return;
> +
> +	tmp = readl(apmixed_base + mt->conf->apmixed_buffer_ctl_reg);
> +	tmp &= mt->conf->apmixed_buffer_ctl_mask;
> +	tmp |= mt->conf->apmixed_buffer_ctl_set;

What is the goal of these two bits operations ?

> +	writel(tmp, apmixed_base + mt->conf->apmixed_buffer_ctl_reg);
>  	udelay(200);
>  }
>  
> @@ -1070,8 +1148,9 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>  		goto err_disable_clk_auxadc;
>  	}
>  
> +	mtk_thermal_turn_on_buffer(mt, apmixed_base);
> +
>  	if (mt->conf->version == MTK_THERMAL_V2) {
> -		mtk_thermal_turn_on_buffer(apmixed_base);
>  		mtk_thermal_release_periodic_ts(mt, auxadc_base);
>  	}
>  
> 


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

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

Re: [PATCH 3/3] thermal: mediatek: add MT8365 thermal driver support

[Markus Schneider-Pargmann]

Hi Daniel,

On Sat, Oct 16, 2021 at 10:06:28PM +0200, Daniel Lezcano wrote:
> On 14/10/2021 15:56, Markus Schneider-Pargmann wrote:
> > From: Fabien Parent <fparent@baylibre.com>
> > 
> > mt8365 is similar to the other SoCs supported by the driver. It has only
> > one bank and 3 sensors that can be multiplexed.
> > 
> > Additionally the buffer has to be enabled and connected to AUXADC
> > similar to the V2 version but at a different register offset. That's why
> > I added three new configuration values to define the register, mask and
> > bits to be set to be able to use it for both V2 and mt8365.
> > 
> > Signed-off-by: Fabien Parent <fparent@baylibre.com>
> > [Added apmixed control register logic]
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> >  drivers/thermal/mtk_thermal.c | 91 ++++++++++++++++++++++++++++++++---
> >  1 file changed, 85 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> > index 93ee043d70da..7a75ae8231f2 100644
> > --- a/drivers/thermal/mtk_thermal.c
> > +++ b/drivers/thermal/mtk_thermal.c
> > @@ -31,6 +31,7 @@
> >  #define AUXADC_CON2_V		0x010
> >  #define AUXADC_DATA(channel)	(0x14 + (channel) * 4)
> >  
> > +#define APMIXED_SYS_TS_CON0	0x600
> >  #define APMIXED_SYS_TS_CON1	0x604
> >  
> >  /* Thermal Controller Registers */
> > @@ -245,6 +246,17 @@ enum mtk_thermal_version {
> >  /* The calibration coefficient of sensor  */
> >  #define MT8183_CALIBRATION	153
> >  
> > +/* MT8365 */
> > +#define MT8365_TEMP_AUXADC_CHANNEL 11
> > +#define MT8365_CALIBRATION 164
> > +#define MT8365_NUM_CONTROLLER 1
> > +#define MT8365_NUM_BANKS 1
> > +#define MT8365_NUM_SENSORS 3
> > +#define MT8365_NUM_SENSORS_PER_ZONE 3
> 
> You can get rid of these macros by using ARRAY_SIZE.
> 
> eg.
> 
> static const int mt8365_bank_data[] = {
> 	MT8365_TS1,
> 	MT8365_TS2,
> 	MT8365_TS3
> };
> 
> ...
> 
> 	.num_sensors = ARRAY_SIZE(mt8365_bank_data);
> ...
> 

Thanks, done, I also used ARRAY_SIZE() on the other array definitions to
enforce that they have the correct size.

> 
> > +#define MT8365_TS1 0
> > +#define MT8365_TS2 1
> > +#define MT8365_TS3 2
> > +
> >  struct mtk_thermal;
> >  
> >  struct thermal_bank_cfg {
> > @@ -271,6 +283,9 @@ struct mtk_thermal_data {
> >  	bool need_switch_bank;
> >  	struct thermal_bank_cfg bank_data[MAX_NUM_ZONES];
> >  	enum mtk_thermal_version version;
> > +	u32 apmixed_buffer_ctl_reg;
> > +	u32 apmixed_buffer_ctl_mask;
> > +	u32 apmixed_buffer_ctl_set;
> >  };
> >  
> >  struct mtk_thermal {
> > @@ -386,6 +401,24 @@ static const int mt7622_mux_values[MT7622_NUM_SENSORS] = { 0, };
> >  static const int mt7622_vts_index[MT7622_NUM_SENSORS] = { VTS1 };
> >  static const int mt7622_tc_offset[MT7622_NUM_CONTROLLER] = { 0x0, };
> >  
> > +/* MT8365 thermal sensor data */
> > +static const int mt8365_bank_data[MT8365_NUM_SENSORS] = {
> > +	MT8365_TS1, MT8365_TS2, MT8365_TS3
> > +};
> > +
> > +static const int mt8365_msr[MT8365_NUM_SENSORS_PER_ZONE] = {
> > +	TEMP_MSR0, TEMP_MSR1, TEMP_MSR2
> > +};
> > +
> > +static const int mt8365_adcpnp[MT8365_NUM_SENSORS_PER_ZONE] = {
> > +	TEMP_ADCPNP0, TEMP_ADCPNP1, TEMP_ADCPNP2
> > +};
> > +
> > +static const int mt8365_mux_values[MT8365_NUM_SENSORS] = { 0, 1, 2 };
> > +static const int mt8365_tc_offset[MT8365_NUM_CONTROLLER] = { 0 };
> > +
> > +static const int mt8365_vts_index[MT8365_NUM_SENSORS] = { VTS1, VTS2, VTS3 };
> > +
> >  /*
> >   * The MT8173 thermal controller has four banks. Each bank can read up to
> >   * four temperature sensors simultaneously. The MT8173 has a total of 5
> > @@ -460,6 +493,40 @@ static const struct mtk_thermal_data mt2701_thermal_data = {
> >  	.version = MTK_THERMAL_V1,
> >  };
> >  
> > +/*
> > + * The MT8365 thermal controller has one bank, which can read up to
> > + * four temperature sensors simultaneously. The MT8365 has a total of 3
> > + * 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 mt8365_thermal_data = {
> > +	.auxadc_channel = MT8365_TEMP_AUXADC_CHANNEL,
> > +	.num_banks = MT8365_NUM_BANKS,
> > +	.num_sensors = MT8365_NUM_SENSORS,
> > +	.vts_index = mt8365_vts_index,
> > +	.cali_val = MT8365_CALIBRATION,
> > +	.num_controller = MT8365_NUM_CONTROLLER,
> > +	.controller_offset = mt8365_tc_offset,
> > +	.need_switch_bank = false,
> > +	.bank_data = {
> > +		{
> > +			.num_sensors = MT8365_NUM_SENSORS,
> > +			.sensors = mt8365_bank_data
> > +		},
> > +	},
> > +	.msr = mt8365_msr,
> > +	.adcpnp = mt8365_adcpnp,
> > +	.sensor_mux_values = mt8365_mux_values,
> > +	.version = MTK_THERMAL_V1,
> > +	.apmixed_buffer_ctl_reg = APMIXED_SYS_TS_CON0,
> > +	.apmixed_buffer_ctl_mask = ~(u32)GENMASK(29, 28),
> > +	.apmixed_buffer_ctl_set = 0,
> > +};
> > +
> >  /*
> >   * The MT2712 thermal controller has one bank, which can read up to
> >   * four temperature sensors simultaneously. The MT2712 has a total of 4
> > @@ -514,6 +581,9 @@ static const struct mtk_thermal_data mt7622_thermal_data = {
> >  	.adcpnp = mt7622_adcpnp,
> >  	.sensor_mux_values = mt7622_mux_values,
> >  	.version = MTK_THERMAL_V2,
> > +	.apmixed_buffer_ctl_reg = APMIXED_SYS_TS_CON1,
> > +	.apmixed_buffer_ctl_mask = ~0x37,
> > +	.apmixed_buffer_ctl_set = 0x1,
> 
> Please change those literal into macros

done.

> 
> >  };
> >  
> >  /*
> > @@ -958,19 +1028,27 @@ static const struct of_device_id mtk_thermal_of_match[] = {
> >  	{
> >  		.compatible = "mediatek,mt8183-thermal",
> >  		.data = (void *)&mt8183_thermal_data,
> > +	},
> > +	{
> > +		.compatible = "mediatek,mt8365-thermal",
> > +		.data = (void *)&mt8365_thermal_data,
> 
> Is this cast really needed ?

No, removed and created another patch to fix all other ones. Would look
weird otherwise.

> 
> >  	}, {
> >  	},
> >  };
> >  MODULE_DEVICE_TABLE(of, mtk_thermal_of_match);
> >  
> > -static void mtk_thermal_turn_on_buffer(void __iomem *apmixed_base)
> > +static void mtk_thermal_turn_on_buffer(struct mtk_thermal *mt,
> > +				       void __iomem *apmixed_base)
> >  {
> >  	int tmp;
> >  
> > -	tmp = readl(apmixed_base + APMIXED_SYS_TS_CON1);
> > -	tmp &= ~(0x37);
> > -	tmp |= 0x1;
> > -	writel(tmp, apmixed_base + APMIXED_SYS_TS_CON1);
> > +	if (!mt->conf->apmixed_buffer_ctl_reg)
> > +		return;
> > +
> > +	tmp = readl(apmixed_base + mt->conf->apmixed_buffer_ctl_reg);
> > +	tmp &= mt->conf->apmixed_buffer_ctl_mask;
> > +	tmp |= mt->conf->apmixed_buffer_ctl_set;
> 
> What is the goal of these two bits operations ?

mt7622 needs to unset a few bits and set one bit in this register.
mt8365 only unsets bits. For this purpose I created a _mask field to
unset bits and a _set field to set bits.

Would you suggest a different way?

Thanks for your feedback.

Best,
Markus

> 
> > +	writel(tmp, apmixed_base + mt->conf->apmixed_buffer_ctl_reg);
> >  	udelay(200);
> >  }
> >  
> > @@ -1070,8 +1148,9 @@ static int mtk_thermal_probe(struct platform_device *pdev)
> >  		goto err_disable_clk_auxadc;
> >  	}
> >  
> > +	mtk_thermal_turn_on_buffer(mt, apmixed_base);
> > +
> >  	if (mt->conf->version == MTK_THERMAL_V2) {
> > -		mtk_thermal_turn_on_buffer(apmixed_base);
> >  		mtk_thermal_release_periodic_ts(mt, auxadc_base);
> >  	}
> >  
> > 
> 
> 
> -- 
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 3/3] thermal: mediatek: add MT8365 thermal driver support

[Daniel Lezcano]

Hi Markus,

On 18/10/2021 17:40, Markus Schneider-Pargmann wrote:

[ ... ]

>>
>>>  	}, {
>>>  	},
>>>  };
>>>  MODULE_DEVICE_TABLE(of, mtk_thermal_of_match);
>>>  
>>> -static void mtk_thermal_turn_on_buffer(void __iomem *apmixed_base)
>>> +static void mtk_thermal_turn_on_buffer(struct mtk_thermal *mt,
>>> +				       void __iomem *apmixed_base)
>>>  {
>>>  	int tmp;
>>>  
>>> -	tmp = readl(apmixed_base + APMIXED_SYS_TS_CON1);
>>> -	tmp &= ~(0x37);
>>> -	tmp |= 0x1;
>>> -	writel(tmp, apmixed_base + APMIXED_SYS_TS_CON1);
>>> +	if (!mt->conf->apmixed_buffer_ctl_reg)
>>> +		return;
>>> +
>>> +	tmp = readl(apmixed_base + mt->conf->apmixed_buffer_ctl_reg);
>>> +	tmp &= mt->conf->apmixed_buffer_ctl_mask;
>>> +	tmp |= mt->conf->apmixed_buffer_ctl_set;
>>
>> What is the goal of these two bits operations ?
> 
> mt7622 needs to unset a few bits and set one bit in this register.
> mt8365 only unsets bits. For this purpose I created a _mask field to
> unset bits and a _set field to set bits.
> 
> Would you suggest a different way?

No, I understand what you wanted to do. I was just unsure if there was a
trick because of the tmp |= 0;




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

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