BPI-R64 Thermal

Hi,

I activated all that stuff in -devices-generic_thermal_sysfs_drivers (specially the MTK things). After reboot I find the expected file /sys/class/thermal/thermal_zone0/temp. But alas - it is empty! Zero degrees!

Am I doing something wrong or should it be true that there is no temperature measuring device? That would be a pity as I wanted to automatically start a Fan when appropriate.

maybe some related modules: Lm-sensors support

which kernel do you use?

here with 5.5-merged:

root@bpi-r2:~# cat /sys/class/thermal/thermal_zone0/temp
48727
root@bpi-r2:~# lsmod
Module                  Size  Used by
ath10k_pci             57344  0
ath10k_core           454656  1 ath10k_pci
ath                    32768  1 ath10k_core
spi_mt65xx             20480  0
mtk_thermal            16384  0
pwm_mediatek           16384  0
mt6577_auxadc          16384  0
mtk_pmic_keys          16384  0
nvmem_mtk_efuse        16384  0
ip_tables              24576  0
x_tables               32768  1 ip_tables

Edit: oh sorry,you talking about r64…not r2

BPI-R64(MT7622) has temperature measuring device.

However, it is found that there are some issues causing cpu-temp read error.

I’ve prepared two sketch patches based on kernel 5.5 rc-1 to solve the known MT7622 thermal issues.

These will be sent to upstream mainline ASAP.

Please apply the following two patches in order.

[1st patch]: modify the common code in order to prepare for adding correct MT7622 thermal support.

 diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index acf4854..31dd1b4 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -120,18 +120,18 @@
  * MT2701 has 3 sensors and needs 3 VTS calibration data.
  * MT2712 has 4 sensors and needs 4 VTS calibration data.
  */
-#define CALIB_BUF0_VALID		BIT(0)
-#define CALIB_BUF1_ADC_GE(x)		(((x) >> 22) & 0x3ff)
-#define CALIB_BUF0_VTS_TS1(x)		(((x) >> 17) & 0x1ff)
-#define CALIB_BUF0_VTS_TS2(x)		(((x) >> 8) & 0x1ff)
-#define CALIB_BUF1_VTS_TS3(x)		(((x) >> 0) & 0x1ff)
-#define CALIB_BUF2_VTS_TS4(x)		(((x) >> 23) & 0x1ff)
-#define CALIB_BUF2_VTS_TS5(x)		(((x) >> 5) & 0x1ff)
-#define CALIB_BUF2_VTS_TSABB(x)		(((x) >> 14) & 0x1ff)
-#define CALIB_BUF0_DEGC_CALI(x)		(((x) >> 1) & 0x3f)
-#define CALIB_BUF0_O_SLOPE(x)		(((x) >> 26) & 0x3f)
-#define CALIB_BUF0_O_SLOPE_SIGN(x)	(((x) >> 7) & 0x1)
-#define CALIB_BUF1_ID(x)		(((x) >> 9) & 0x1)
+#define CALIB_BUF0_VALID_V1		BIT(0)
+#define CALIB_BUF1_ADC_GE_V1(x)		(((x) >> 22) & 0x3ff)
+#define CALIB_BUF0_VTS_TS1_V1(x)	(((x) >> 17) & 0x1ff)
+#define CALIB_BUF0_VTS_TS2_V1(x)	(((x) >> 8) & 0x1ff)
+#define CALIB_BUF1_VTS_TS3_V1(x)	(((x) >> 0) & 0x1ff)
+#define CALIB_BUF2_VTS_TS4_V1(x)	(((x) >> 23) & 0x1ff)
+#define CALIB_BUF2_VTS_TS5_V1(x)	(((x) >> 5) & 0x1ff)
+#define CALIB_BUF2_VTS_TSABB_V1(x)	(((x) >> 14) & 0x1ff)
+#define CALIB_BUF0_DEGC_CALI_V1(x)	(((x) >> 1) & 0x3f)
+#define CALIB_BUF0_O_SLOPE_V1(x)	(((x) >> 26) & 0x3f)
+#define CALIB_BUF0_O_SLOPE_SIGN_V1(x)	(((x) >> 7) & 0x1)
+#define CALIB_BUF1_ID_V1(x)		(((x) >> 9) & 0x1)
 
 enum {
 	VTS1,
@@ -143,6 +143,11 @@
 	MAX_NUM_VTS,
 };
 
+enum mtk_thermal_version {
+	MTK_THERMAL_V1 = 1,
+	MTK_THERMAL_V2,
+};
+
 /* MT2701 thermal sensors */
 #define MT2701_TS1	0
 #define MT2701_TS2	1
@@ -245,6 +250,9 @@
 	const int *controller_offset;
 	bool need_switch_bank;
 	struct thermal_bank_cfg bank_data[MAX_NUM_ZONES];
+	enum mtk_thermal_version version;
+	int (*extract)(struct mtk_thermal *mt, u32 *buf);
+	int (*convert)(struct mtk_thermal *mt, int sensno, s32 raw);
 };
 
 struct mtk_thermal {
@@ -358,6 +366,9 @@
 static const int mt7622_vts_index[MT7622_NUM_SENSORS] = { VTS1 };
 static const int mt7622_tc_offset[MT7622_NUM_CONTROLLER] = { 0x0, };
 
+static int mtk_thermal_extract_efuse_v1(struct mtk_thermal *mt, u32 *buf);
+static int raw_to_mcelsius_v1(struct mtk_thermal *mt, int sensno, s32 raw);
+
 /**
  * The MT8173 thermal controller has four banks. Each bank can read up to
  * four temperature sensors simultaneously. The MT8173 has a total of 5
@@ -398,6 +409,9 @@
 	.msr = mt8173_msr,
 	.adcpnp = mt8173_adcpnp,
 	.sensor_mux_values = mt8173_mux_values,
+	.version = MTK_THERMAL_V1,
+	.extract = mtk_thermal_extract_efuse_v1,
+	.convert = raw_to_mcelsius_v1,
 };
 
 /**
@@ -428,6 +442,9 @@
 	.msr = mt2701_msr,
 	.adcpnp = mt2701_adcpnp,
 	.sensor_mux_values = mt2701_mux_values,
+	.version = MTK_THERMAL_V1,
+	.extract = mtk_thermal_extract_efuse_v1,
+	.convert = raw_to_mcelsius_v1,
 };
 
 /**
@@ -458,6 +475,9 @@
 	.msr = mt2712_msr,
 	.adcpnp = mt2712_adcpnp,
 	.sensor_mux_values = mt2712_mux_values,
+	.version = MTK_THERMAL_V1,
+	.extract = mtk_thermal_extract_efuse_v1,
+	.convert = raw_to_mcelsius_v1,
 };
 
 /*
@@ -482,6 +502,9 @@
 	.msr = mt7622_msr,
 	.adcpnp = mt7622_adcpnp,
 	.sensor_mux_values = mt7622_mux_values,
+	.version = MTK_THERMAL_V2,
+	.extract = mtk_thermal_extract_efuse_v1,
+	.convert = raw_to_mcelsius_v1,
 };
 
 /**
@@ -515,6 +538,9 @@
 	.msr = mt8183_msr,
 	.adcpnp = mt8183_adcpnp,
 	.sensor_mux_values = mt8183_mux_values,
+	.version = MTK_THERMAL_V1,
+	.extract = mtk_thermal_extract_efuse_v1,
+	.convert = raw_to_mcelsius_v1,
 };
 
 /**
@@ -525,7 +551,7 @@
  * This converts the raw ADC value to mcelsius using the SoC specific
  * calibration constants
  */
-static int raw_to_mcelsius(struct mtk_thermal *mt, int sensno, s32 raw)
+static int raw_to_mcelsius_v1(struct mtk_thermal *mt, int sensno, s32 raw)
 {
 	s32 tmp;
 
@@ -594,9 +620,9 @@
 		raw = readl(mt->thermal_base +
 			    conf->msr[conf->bank_data[bank->id].sensors[i]]);
 
-		temp = raw_to_mcelsius(mt,
-				       conf->bank_data[bank->id].sensors[i],
-				       raw);
+		temp = conf->convert(mt,
+				     conf->bank_data[bank->id].sensors[i],
+				     raw);
 
 		/*
 		 * The first read of a sensor often contains very high bogus
@@ -698,9 +724,11 @@
 	writel(auxadc_phys_base + AUXADC_CON1_CLR_V,
 	       controller_base + TEMP_ADCMUXADDR);
 
-	/* AHB address for pnp sensor mux selection */
-	writel(apmixed_phys_base + APMIXED_SYS_TS_CON1,
-	       controller_base + TEMP_PNPMUXADDR);
+	if (mt->conf->version == MTK_THERMAL_V1) {
+		/* AHB address for pnp sensor mux selection */
+		writel(apmixed_phys_base + APMIXED_SYS_TS_CON1,
+		       controller_base + TEMP_PNPMUXADDR);
+	}
 
 	/* AHB value for auxadc enable */
 	writel(BIT(conf->auxadc_channel), controller_base + TEMP_ADCEN);
@@ -758,6 +786,51 @@
 	return of_translate_address(np, regaddr_p);
 }
 
+static int mtk_thermal_extract_efuse_v1(struct mtk_thermal *mt, u32 *buf)
+{
+	int i;
+
+	if (!(buf[0] & CALIB_BUF0_VALID_V1))
+		return -EINVAL;
+
+	mt->adc_ge = CALIB_BUF1_ADC_GE_V1(buf[1]);
+
+	for (i = 0; i < mt->conf->num_sensors; i++) {
+		switch (mt->conf->vts_index[i]) {
+		case VTS1:
+			mt->vts[VTS1] = CALIB_BUF0_VTS_TS1_V1(buf[0]);
+			break;
+		case VTS2:
+			mt->vts[VTS2] = CALIB_BUF0_VTS_TS2_V1(buf[0]);
+			break;
+		case VTS3:
+			mt->vts[VTS3] = CALIB_BUF1_VTS_TS3_V1(buf[1]);
+			break;
+		case VTS4:
+			mt->vts[VTS4] = CALIB_BUF2_VTS_TS4_V1(buf[2]);
+			break;
+		case VTS5:
+			mt->vts[VTS5] = CALIB_BUF2_VTS_TS5_V1(buf[2]);
+			break;
+		case VTSABB:
+			mt->vts[VTSABB] =
+				CALIB_BUF2_VTS_TSABB_V1(buf[2]);
+			break;
+		default:
+			break;
+		}
+	}
+
+	mt->degc_cali = CALIB_BUF0_DEGC_CALI_V1(buf[0]);
+	if (CALIB_BUF1_ID_V1(buf[1]) &
+	    CALIB_BUF0_O_SLOPE_SIGN_V1(buf[0]))
+		mt->o_slope = -CALIB_BUF0_O_SLOPE_V1(buf[0]);
+	else
+		mt->o_slope = CALIB_BUF0_O_SLOPE_V1(buf[0]);
+
+	return 0;
+}
+
 static int mtk_thermal_get_calibration_data(struct device *dev,
 					    struct mtk_thermal *mt)
 {
@@ -793,42 +866,9 @@
 		goto out;
 	}
 
-	if (buf[0] & CALIB_BUF0_VALID) {
-		mt->adc_ge = CALIB_BUF1_ADC_GE(buf[1]);
-
-		for (i = 0; i < mt->conf->num_sensors; i++) {
-			switch (mt->conf->vts_index[i]) {
-			case VTS1:
-				mt->vts[VTS1] = CALIB_BUF0_VTS_TS1(buf[0]);
-				break;
-			case VTS2:
-				mt->vts[VTS2] = CALIB_BUF0_VTS_TS2(buf[0]);
-				break;
-			case VTS3:
-				mt->vts[VTS3] = CALIB_BUF1_VTS_TS3(buf[1]);
-				break;
-			case VTS4:
-				mt->vts[VTS4] = CALIB_BUF2_VTS_TS4(buf[2]);
-				break;
-			case VTS5:
-				mt->vts[VTS5] = CALIB_BUF2_VTS_TS5(buf[2]);
-				break;
-			case VTSABB:
-				mt->vts[VTSABB] = CALIB_BUF2_VTS_TSABB(buf[2]);
-				break;
-			default:
-				break;
-			}
-		}
-
-		mt->degc_cali = CALIB_BUF0_DEGC_CALI(buf[0]);
-		if (CALIB_BUF1_ID(buf[1]) &
-		    CALIB_BUF0_O_SLOPE_SIGN(buf[0]))
-			mt->o_slope = -CALIB_BUF0_O_SLOPE(buf[0]);
-		else
-			mt->o_slope = CALIB_BUF0_O_SLOPE(buf[0]);
-	} else {
+	if (mt->conf->extract(mt, buf)) {
 		dev_info(dev, "Device not calibrated, using default calibration values\n");
+		ret = -EINVAL;
 	}
 
 out:

[2nd patch]: refine MT7622-specific settings to fix cpu-temp read error

  • hardware init flow
  • efuse layout
  • raw_to_mcelsius formula

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index 31dd1b4..65a86af 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -38,6 +38,7 @@
 #define TEMP_MONIDET0		0x014
 #define TEMP_MONIDET1		0x018
 #define TEMP_MSRCTL0		0x038
+#define TEMP_MSRCTL1		0x03c
 #define TEMP_AHBPOLL		0x040
 #define TEMP_AHBTO		0x044
 #define TEMP_ADCPNP0		0x048
@@ -133,6 +134,20 @@
 #define CALIB_BUF0_O_SLOPE_SIGN_V1(x)	(((x) >> 7) & 0x1)
 #define CALIB_BUF1_ID_V1(x)		(((x) >> 9) & 0x1)
 
+/*
+ * Layout of the fuses providing the calibration data
+ * These macros could be used for MT7622.
+ */
+#define CALIB_BUF0_ADC_OE_V2(x)		(((x) >> 22) & 0x3ff)
+#define CALIB_BUF0_ADC_GE_V2(x)		(((x) >> 12) & 0x3ff)
+#define CALIB_BUF0_DEGC_CALI_V2(x)	(((x) >> 6) & 0x3f)
+#define CALIB_BUF0_O_SLOPE_V2(x)	(((x) >> 0) & 0x3f)
+#define CALIB_BUF1_VTS_TS1_V2(x)	(((x) >> 23) & 0x1ff)
+#define CALIB_BUF1_VTS_TS2_V2(x)	(((x) >> 14) & 0x1ff)
+#define CALIB_BUF1_VTS_TSABB_V2(x)	(((x) >> 5) & 0x1ff)
+#define CALIB_BUF1_VALID_V2(x)		(((x) >> 4) & 0x1)
+#define CALIB_BUF1_O_SLOPE_SIGN_V2(x)	(((x) >> 3) & 0x1)
+
 enum {
 	VTS1,
 	VTS2,
@@ -266,8 +281,10 @@
 
 	/* Calibration values */
 	s32 adc_ge;
+	s32 adc_oe;
 	s32 degc_cali;
 	s32 o_slope;
+	s32 o_slope_sign;
 	s32 vts[MAX_NUM_VTS];
 
 	const struct mtk_thermal_data *conf;
@@ -367,7 +384,9 @@
 static const int mt7622_tc_offset[MT7622_NUM_CONTROLLER] = { 0x0, };
 
 static int mtk_thermal_extract_efuse_v1(struct mtk_thermal *mt, u32 *buf);
+static int mtk_thermal_extract_efuse_v2(struct mtk_thermal *mt, u32 *buf);
 static int raw_to_mcelsius_v1(struct mtk_thermal *mt, int sensno, s32 raw);
+static int raw_to_mcelsius_v2(struct mtk_thermal *mt, int sensno, s32 raw);
 
 /**
  * The MT8173 thermal controller has four banks. Each bank can read up to
@@ -503,8 +522,8 @@
 	.adcpnp = mt7622_adcpnp,
 	.sensor_mux_values = mt7622_mux_values,
 	.version = MTK_THERMAL_V2,
-	.extract = mtk_thermal_extract_efuse_v1,
-	.convert = raw_to_mcelsius_v1,
+	.extract = mtk_thermal_extract_efuse_v2,
+	.convert = raw_to_mcelsius_v2,
 };
 
 /**
@@ -566,6 +585,36 @@
 	return mt->degc_cali * 500 - tmp;
 }
 
+static int raw_to_mcelsius_v2(struct mtk_thermal *mt, int sensno, s32 raw)
+{
+	s32 format_1 = 0;
+	s32 format_2 = 0;
+	s32 g_oe = 1;
+	s32 g_gain = 1;
+	s32 g_x_roomt = 0;
+	s32 tmp = 0;
+
+	if (raw == 0)
+		return 0;
+
+	raw &= 0xfff;
+	g_gain = 10000 + (((mt->adc_ge - 512) * 10000) >> 12);
+	g_oe = mt->adc_oe - 512;
+	format_1 = mt->vts[VTS2] + 3105 - g_oe;
+	format_2 = (mt->degc_cali * 10) >> 1;
+	g_x_roomt = (((format_1 * 10000) >> 12) * 10000) / g_gain;
+
+	tmp = (((((raw - g_oe) * 10000) >> 12) * 10000) / g_gain) - g_x_roomt;
+	tmp = tmp * 10 * 100 / 11;
+
+	if (mt->o_slope_sign == 0)
+		tmp = tmp / (165 - mt->o_slope);
+	else
+		tmp = tmp / (165 + mt->o_slope);
+
+	return (format_2 - tmp) * 100;
+}
+
 /**
  * mtk_thermal_get_bank - get bank
  * @bank:	The bank
@@ -831,6 +880,23 @@
 	return 0;
 }
 
+static int mtk_thermal_extract_efuse_v2(struct mtk_thermal *mt, u32 *buf)
+{
+	if (!CALIB_BUF1_VALID_V2(buf[1]))
+		return -EINVAL;
+
+	mt->adc_oe = CALIB_BUF0_ADC_OE_V2(buf[0]);
+	mt->adc_ge = CALIB_BUF0_ADC_GE_V2(buf[0]);
+	mt->degc_cali = CALIB_BUF0_DEGC_CALI_V2(buf[0]);
+	mt->o_slope = CALIB_BUF0_O_SLOPE_V2(buf[0]);
+	mt->vts[VTS1] = CALIB_BUF1_VTS_TS1_V2(buf[1]);
+	mt->vts[VTS2] = CALIB_BUF1_VTS_TS2_V2(buf[1]);
+	mt->vts[VTSABB] = CALIB_BUF1_VTS_TSABB_V2(buf[1]);
+	mt->o_slope_sign = CALIB_BUF1_O_SLOPE_SIGN_V2(buf[1]);
+
+	return 0;
+}
+
 static int mtk_thermal_get_calibration_data(struct device *dev,
 					    struct mtk_thermal *mt)
 {
@@ -902,6 +968,28 @@
 };
 MODULE_DEVICE_TABLE(of, mtk_thermal_of_match);
 
+static void mtk_thermal_turn_on_buffer(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);
+	udelay(200);
+}
+
+static void mtk_thermal_release_periodic_ts(struct mtk_thermal *mt,
+					    void __iomem *auxadc_base)
+{
+	int tmp;
+
+	writel(0x800, auxadc_base + AUXADC_CON1_SET_V);
+	writel(0x1, mt->thermal_base + TEMP_MONCTL0);
+	tmp = readl(mt->thermal_base + TEMP_MSRCTL1);
+	writel((tmp & (~0x10e)), mt->thermal_base + TEMP_MSRCTL1);
+}
+
 static int mtk_thermal_probe(struct platform_device *pdev)
 {
 	int ret, i, ctrl_id;
@@ -910,6 +998,7 @@
 	struct resource *res;
 	u64 auxadc_phys_base, apmixed_phys_base;
 	struct thermal_zone_device *tzdev;
+	void __iomem *apmixed_base, *auxadc_base;
 
 	mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
 	if (!mt)
@@ -944,6 +1033,7 @@
 		return -ENODEV;
 	}
 
+	auxadc_base = of_iomap(auxadc, 0);
 	auxadc_phys_base = of_get_phys_base(auxadc);
 
 	of_node_put(auxadc);
@@ -959,6 +1049,7 @@
 		return -ENODEV;
 	}
 
+	apmixed_base = of_iomap(apmixedsys, 0);
 	apmixed_phys_base = of_get_phys_base(apmixedsys);
 
 	of_node_put(apmixedsys);
@@ -984,6 +1075,11 @@
 		goto err_disable_clk_auxadc;
 	}
 
+	if (mt->conf->version == MTK_THERMAL_V2) {
+		mtk_thermal_turn_on_buffer(apmixed_base);
+		mtk_thermal_release_periodic_ts(mt, auxadc_base);
+	}
+
 	for (ctrl_id = 0; ctrl_id < mt->conf->num_controller ; ctrl_id++)
 		for (i = 0; i < mt->conf->num_banks; i++)
 			mtk_thermal_init_bank(mt, i, apmixed_phys_base,
1 Like

thank you henry

have added your patches to my 5.4-r64-main branch, but not yet tested

edit: seems working:

root@bpi-r64:~# cat /sys/class/thermal/thermal_zone0/temp                       
36700                                                                           
root@bpi-r64:~#
1 Like

Thanks much, Henry and Frank!

I’ll give it a try.

@ntech does it work for you?

@henry Maybe you can send me your fullname+email via pm so i can set author-information in git.

Sorry, couldn’t test it by now. I hope I can test it next weekend.

hi @henry seems mt7622 thermal patches breaking mt7623 thermal…

on my old 5.4-main branch thermal was working…on new (where i merged r64 patches) it gives invalid argument

dmesg shows this:

mtk-thermal: probe of 1100b000.thermal failed with error -22

see [BPI-R2] Kernel Development

error is generated here:

 838 static int mtk_thermal_extract_efuse_v1(struct mtk_thermal *mt, u32 *buf)
 839 {
 840         int i;
 841 printk(KERN_ALERT "DEBUG: Passed %s %d \n",__FUNCTION__,__LINE__);
 842         if (!(buf[0] & CALIB_BUF0_VALID_V1))
 843                 return -EINVAL;
 844 printk(KERN_ALERT "DEBUG: Passed %s %d \n",__FUNCTION__,__LINE__);

dmesg (line 844 is not printed):

root@bpi-r2:~# modprobe mtk_thermal
[  619.150578] DEBUG: Passed mtk_thermal_get_calibration_data 929 
[  619.156518] DEBUG: Passed mtk_thermal_get_calibration_data 935 
[  619.162478] DEBUG: Passed mtk_thermal_extract_efuse_v1 841 
[  619.168091] mtk-thermal 1100b000.thermal: Device not calibrated, using default calibration values
[  619.176959] DEBUG: Passed mtk_thermal_get_calibration_data 941 
[  619.182949] mtk-thermal: probe of 1100b000.thermal failed with error -22

values checked:

0x7a254000 0x1 0x0 = buf[0],CALIB_BUF0_VALID_V1,(buf[0] & CALIB_BUF0_VALID_V1)

before BUF0_VALID was only used in mtk_thermal_get_calibration_data but code was moved to extract-function which is called in get-calibration-data…

i guess this is the problem:

+	if (mt->conf->extract(mt, buf)) {
 		dev_info(dev, "Device not calibrated, using default calibration values\n");
+		ret = -EINVAL;
 	}

before data is only read if valid and if not only message is printed…now mtk_thermal_get_calibration_data is exited with EINVAL in this case…

i disabled only this “ret=-EINVAL” and have same behaviour as without Patch…so better as reverting full Patches

@frank-w
Thanks for your report !
Indeed “ret=-EINVAL” is an unnecessary action. So I revised my patches as follows.
Thanks.

Please apply the following two patches in order.

[1st patch]: modify the common code in order to prepare for adding correct MT7622 thermal support.

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index acf4854cbb8b..2d122a11d524 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -120,18 +120,18 @@
  * MT2701 has 3 sensors and needs 3 VTS calibration data.
  * MT2712 has 4 sensors and needs 4 VTS calibration data.
  */
-#define CALIB_BUF0_VALID       BIT(0)
-#define CALIB_BUF1_ADC_GE(x)       (((x) >> 22) & 0x3ff)
-#define CALIB_BUF0_VTS_TS1(x)      (((x) >> 17) & 0x1ff)
-#define CALIB_BUF0_VTS_TS2(x)      (((x) >> 8) & 0x1ff)
-#define CALIB_BUF1_VTS_TS3(x)      (((x) >> 0) & 0x1ff)
-#define CALIB_BUF2_VTS_TS4(x)      (((x) >> 23) & 0x1ff)
-#define CALIB_BUF2_VTS_TS5(x)      (((x) >> 5) & 0x1ff)
-#define CALIB_BUF2_VTS_TSABB(x)        (((x) >> 14) & 0x1ff)
-#define CALIB_BUF0_DEGC_CALI(x)        (((x) >> 1) & 0x3f)
-#define CALIB_BUF0_O_SLOPE(x)      (((x) >> 26) & 0x3f)
-#define CALIB_BUF0_O_SLOPE_SIGN(x) (((x) >> 7) & 0x1)
-#define CALIB_BUF1_ID(x)       (((x) >> 9) & 0x1)
+#define CALIB_BUF0_VALID_V1        BIT(0)
+#define CALIB_BUF1_ADC_GE_V1(x)        (((x) >> 22) & 0x3ff)
+#define CALIB_BUF0_VTS_TS1_V1(x)   (((x) >> 17) & 0x1ff)
+#define CALIB_BUF0_VTS_TS2_V1(x)   (((x) >> 8) & 0x1ff)
+#define CALIB_BUF1_VTS_TS3_V1(x)   (((x) >> 0) & 0x1ff)
+#define CALIB_BUF2_VTS_TS4_V1(x)   (((x) >> 23) & 0x1ff)
+#define CALIB_BUF2_VTS_TS5_V1(x)   (((x) >> 5) & 0x1ff)
+#define CALIB_BUF2_VTS_TSABB_V1(x) (((x) >> 14) & 0x1ff)
+#define CALIB_BUF0_DEGC_CALI_V1(x) (((x) >> 1) & 0x3f)
+#define CALIB_BUF0_O_SLOPE_V1(x)   (((x) >> 26) & 0x3f)
+#define CALIB_BUF0_O_SLOPE_SIGN_V1(x)  (((x) >> 7) & 0x1)
+#define CALIB_BUF1_ID_V1(x)        (((x) >> 9) & 0x1)
 
 enum {
    VTS1,
@@ -143,6 +143,11 @@ enum {
    MAX_NUM_VTS,
 };
 
+enum mtk_thermal_version {
+   MTK_THERMAL_V1 = 1,
+   MTK_THERMAL_V2,
+};
+
 /* MT2701 thermal sensors */
 #define MT2701_TS1 0
 #define MT2701_TS2 1
@@ -245,6 +250,9 @@ struct mtk_thermal_data {
    const int *controller_offset;
    bool need_switch_bank;
    struct thermal_bank_cfg bank_data[MAX_NUM_ZONES];
+   enum mtk_thermal_version version;
+   int (*extract)(struct mtk_thermal *mt, u32 *buf);
+   int (*convert)(struct mtk_thermal *mt, int sensno, s32 raw);
 };
 
 struct mtk_thermal {
@@ -358,6 +366,9 @@ 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, };
 
+static int mtk_thermal_extract_efuse_v1(struct mtk_thermal *mt, u32 *buf);
+static int raw_to_mcelsius_v1(struct mtk_thermal *mt, int sensno, s32 raw);
+
 /**
  * The MT8173 thermal controller has four banks. Each bank can read up to
  * four temperature sensors simultaneously. The MT8173 has a total of 5
@@ -398,6 +409,9 @@ static const struct mtk_thermal_data mt8173_thermal_data = {
    .msr = mt8173_msr,
    .adcpnp = mt8173_adcpnp,
    .sensor_mux_values = mt8173_mux_values,
+   .version = MTK_THERMAL_V1,
+   .extract = mtk_thermal_extract_efuse_v1,
+   .convert = raw_to_mcelsius_v1,
 };
 
 /**
@@ -428,6 +442,9 @@ static const struct mtk_thermal_data mt2701_thermal_data = {
    .msr = mt2701_msr,
    .adcpnp = mt2701_adcpnp,
    .sensor_mux_values = mt2701_mux_values,
+   .version = MTK_THERMAL_V1,
+   .extract = mtk_thermal_extract_efuse_v1,
+   .convert = raw_to_mcelsius_v1,
 };
 
 /**
@@ -458,6 +475,9 @@ static const struct mtk_thermal_data mt2712_thermal_data = {
    .msr = mt2712_msr,
    .adcpnp = mt2712_adcpnp,
    .sensor_mux_values = mt2712_mux_values,
+   .version = MTK_THERMAL_V1,
+   .extract = mtk_thermal_extract_efuse_v1,
+   .convert = raw_to_mcelsius_v1,
 };
 
 /*
@@ -482,6 +502,9 @@ static const struct mtk_thermal_data mt7622_thermal_data = {
    .msr = mt7622_msr,
    .adcpnp = mt7622_adcpnp,
    .sensor_mux_values = mt7622_mux_values,
+   .version = MTK_THERMAL_V2,
+   .extract = mtk_thermal_extract_efuse_v1,
+   .convert = raw_to_mcelsius_v1,
 };
 
 /**
@@ -515,6 +538,9 @@ static const struct mtk_thermal_data mt8183_thermal_data = {
    .msr = mt8183_msr,
    .adcpnp = mt8183_adcpnp,
    .sensor_mux_values = mt8183_mux_values,
+   .version = MTK_THERMAL_V1,
+   .extract = mtk_thermal_extract_efuse_v1,
+   .convert = raw_to_mcelsius_v1,
 };
 
 /**
@@ -525,7 +551,7 @@ static const struct mtk_thermal_data mt8183_thermal_data = {
  * This converts the raw ADC value to mcelsius using the SoC specific
  * calibration constants
  */
-static int raw_to_mcelsius(struct mtk_thermal *mt, int sensno, s32 raw)
+static int raw_to_mcelsius_v1(struct mtk_thermal *mt, int sensno, s32 raw)
 {
    s32 tmp;
 
@@ -594,9 +620,9 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
        raw = readl(mt->thermal_base +
                conf->msr[conf->bank_data[bank->id].sensors[i]]);
 
-       temp = raw_to_mcelsius(mt,
-                      conf->bank_data[bank->id].sensors[i],
-                      raw);
+       temp = conf->convert(mt,
+                    conf->bank_data[bank->id].sensors[i],
+                    raw);
 
        /*
         * The first read of a sensor often contains very high bogus
@@ -698,9 +724,11 @@ static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
    writel(auxadc_phys_base + AUXADC_CON1_CLR_V,
           controller_base + TEMP_ADCMUXADDR);
 
-   /* AHB address for pnp sensor mux selection */
-   writel(apmixed_phys_base + APMIXED_SYS_TS_CON1,
-          controller_base + TEMP_PNPMUXADDR);
+   if (mt->conf->version == MTK_THERMAL_V1) {
+       /* AHB address for pnp sensor mux selection */
+       writel(apmixed_phys_base + APMIXED_SYS_TS_CON1,
+              controller_base + TEMP_PNPMUXADDR);
+   }
 
    /* AHB value for auxadc enable */
    writel(BIT(conf->auxadc_channel), controller_base + TEMP_ADCEN);
@@ -758,6 +786,51 @@ static u64 of_get_phys_base(struct device_node *np)
    return of_translate_address(np, regaddr_p);
 }
 
+static int mtk_thermal_extract_efuse_v1(struct mtk_thermal *mt, u32 *buf)
+{
+   int i;
+
+   if (!(buf[0] & CALIB_BUF0_VALID_V1))
+       return -EINVAL;
+
+   mt->adc_ge = CALIB_BUF1_ADC_GE_V1(buf[1]);
+
+   for (i = 0; i < mt->conf->num_sensors; i++) {
+       switch (mt->conf->vts_index[i]) {
+       case VTS1:
+           mt->vts[VTS1] = CALIB_BUF0_VTS_TS1_V1(buf[0]);
+           break;
+       case VTS2:
+           mt->vts[VTS2] = CALIB_BUF0_VTS_TS2_V1(buf[0]);
+           break;
+       case VTS3:
+           mt->vts[VTS3] = CALIB_BUF1_VTS_TS3_V1(buf[1]);
+           break;
+       case VTS4:
+           mt->vts[VTS4] = CALIB_BUF2_VTS_TS4_V1(buf[2]);
+           break;
+       case VTS5:
+           mt->vts[VTS5] = CALIB_BUF2_VTS_TS5_V1(buf[2]);
+           break;
+       case VTSABB:
+           mt->vts[VTSABB] =
+               CALIB_BUF2_VTS_TSABB_V1(buf[2]);
+           break;
+       default:
+           break;
+       }
+   }
+
+   mt->degc_cali = CALIB_BUF0_DEGC_CALI_V1(buf[0]);
+   if (CALIB_BUF1_ID_V1(buf[1]) &
+       CALIB_BUF0_O_SLOPE_SIGN_V1(buf[0]))
+       mt->o_slope = -CALIB_BUF0_O_SLOPE_V1(buf[0]);
+   else
+       mt->o_slope = CALIB_BUF0_O_SLOPE_V1(buf[0]);
+
+   return 0;
+}
+
 static int mtk_thermal_get_calibration_data(struct device *dev,
                        struct mtk_thermal *mt)
 {
@@ -793,43 +866,8 @@ static int mtk_thermal_get_calibration_data(struct device *dev,
        goto out;
    }
 
-   if (buf[0] & CALIB_BUF0_VALID) {
-       mt->adc_ge = CALIB_BUF1_ADC_GE(buf[1]);
-
-       for (i = 0; i < mt->conf->num_sensors; i++) {
-           switch (mt->conf->vts_index[i]) {
-           case VTS1:
-               mt->vts[VTS1] = CALIB_BUF0_VTS_TS1(buf[0]);
-               break;
-           case VTS2:
-               mt->vts[VTS2] = CALIB_BUF0_VTS_TS2(buf[0]);
-               break;
-           case VTS3:
-               mt->vts[VTS3] = CALIB_BUF1_VTS_TS3(buf[1]);
-               break;
-           case VTS4:
-               mt->vts[VTS4] = CALIB_BUF2_VTS_TS4(buf[2]);
-               break;
-           case VTS5:
-               mt->vts[VTS5] = CALIB_BUF2_VTS_TS5(buf[2]);
-               break;
-           case VTSABB:
-               mt->vts[VTSABB] = CALIB_BUF2_VTS_TSABB(buf[2]);
-               break;
-           default:
-               break;
-           }
-       }
-
-       mt->degc_cali = CALIB_BUF0_DEGC_CALI(buf[0]);
-       if (CALIB_BUF1_ID(buf[1]) &
-           CALIB_BUF0_O_SLOPE_SIGN(buf[0]))
-           mt->o_slope = -CALIB_BUF0_O_SLOPE(buf[0]);
-       else
-           mt->o_slope = CALIB_BUF0_O_SLOPE(buf[0]);
-   } else {
+   if (mt->conf->extract(mt, buf))
        dev_info(dev, "Device not calibrated, using default calibration values\n");
-   }
 
 out:
    kfree(buf);

[2nd patch]: refine MT7622-specific settings to fix cpu-temp read error

  • hardware init flow
  • efuse layout
  • raw_to_mcelsius formula

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index 2d122a11d524..dbee5c0792b0 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -38,6 +38,7 @@
 #define TEMP_MONIDET0      0x014
 #define TEMP_MONIDET1      0x018
 #define TEMP_MSRCTL0       0x038
+#define TEMP_MSRCTL1       0x03c
 #define TEMP_AHBPOLL       0x040
 #define TEMP_AHBTO     0x044
 #define TEMP_ADCPNP0       0x048
@@ -133,6 +134,20 @@
 #define CALIB_BUF0_O_SLOPE_SIGN_V1(x)  (((x) >> 7) & 0x1)
 #define CALIB_BUF1_ID_V1(x)        (((x) >> 9) & 0x1)
 
+/*
+ * Layout of the fuses providing the calibration data
+ * These macros could be used for MT7622.
+ */
+#define CALIB_BUF0_ADC_OE_V2(x)        (((x) >> 22) & 0x3ff)
+#define CALIB_BUF0_ADC_GE_V2(x)        (((x) >> 12) & 0x3ff)
+#define CALIB_BUF0_DEGC_CALI_V2(x) (((x) >> 6) & 0x3f)
+#define CALIB_BUF0_O_SLOPE_V2(x)   (((x) >> 0) & 0x3f)
+#define CALIB_BUF1_VTS_TS1_V2(x)   (((x) >> 23) & 0x1ff)
+#define CALIB_BUF1_VTS_TS2_V2(x)   (((x) >> 14) & 0x1ff)
+#define CALIB_BUF1_VTS_TSABB_V2(x) (((x) >> 5) & 0x1ff)
+#define CALIB_BUF1_VALID_V2(x)     (((x) >> 4) & 0x1)
+#define CALIB_BUF1_O_SLOPE_SIGN_V2(x)  (((x) >> 3) & 0x1)
+
 enum {
    VTS1,
    VTS2,
@@ -266,8 +281,10 @@ struct mtk_thermal {
 
    /* Calibration values */
    s32 adc_ge;
+   s32 adc_oe;
    s32 degc_cali;
    s32 o_slope;
+   s32 o_slope_sign;
    s32 vts[MAX_NUM_VTS];
 
    const struct mtk_thermal_data *conf;
@@ -367,7 +384,9 @@ static const int mt7622_vts_index[MT7622_NUM_SENSORS] = { VTS1 };
 static const int mt7622_tc_offset[MT7622_NUM_CONTROLLER] = { 0x0, };
 
 static int mtk_thermal_extract_efuse_v1(struct mtk_thermal *mt, u32 *buf);
+static int mtk_thermal_extract_efuse_v2(struct mtk_thermal *mt, u32 *buf);
 static int raw_to_mcelsius_v1(struct mtk_thermal *mt, int sensno, s32 raw);
+static int raw_to_mcelsius_v2(struct mtk_thermal *mt, int sensno, s32 raw);
 
 /**
  * The MT8173 thermal controller has four banks. Each bank can read up to
@@ -503,8 +522,8 @@ static const struct mtk_thermal_data mt7622_thermal_data = {
    .adcpnp = mt7622_adcpnp,
    .sensor_mux_values = mt7622_mux_values,
    .version = MTK_THERMAL_V2,
-   .extract = mtk_thermal_extract_efuse_v1,
-   .convert = raw_to_mcelsius_v1,
+   .extract = mtk_thermal_extract_efuse_v2,
+   .convert = raw_to_mcelsius_v2,
 };
 
 /**
@@ -566,6 +585,36 @@ static int raw_to_mcelsius_v1(struct mtk_thermal *mt, int sensno, s32 raw)
    return mt->degc_cali * 500 - tmp;
 }
 
+static int raw_to_mcelsius_v2(struct mtk_thermal *mt, int sensno, s32 raw)
+{
+   s32 format_1 = 0;
+   s32 format_2 = 0;
+   s32 g_oe = 1;
+   s32 g_gain = 1;
+   s32 g_x_roomt = 0;
+   s32 tmp = 0;
+
+   if (raw == 0)
+       return 0;
+
+   raw &= 0xfff;
+   g_gain = 10000 + (((mt->adc_ge - 512) * 10000) >> 12);
+   g_oe = mt->adc_oe - 512;
+   format_1 = mt->vts[VTS2] + 3105 - g_oe;
+   format_2 = (mt->degc_cali * 10) >> 1;
+   g_x_roomt = (((format_1 * 10000) >> 12) * 10000) / g_gain;
+
+   tmp = (((((raw - g_oe) * 10000) >> 12) * 10000) / g_gain) - g_x_roomt;
+   tmp = tmp * 10 * 100 / 11;
+
+   if (mt->o_slope_sign == 0)
+       tmp = tmp / (165 - mt->o_slope);
+   else
+       tmp = tmp / (165 + mt->o_slope);
+
+   return (format_2 - tmp) * 100;
+}
+
 /**
  * mtk_thermal_get_bank - get bank
  * @bank:  The bank
@@ -831,6 +880,23 @@ static int mtk_thermal_extract_efuse_v1(struct mtk_thermal *mt, u32 *buf)
    return 0;
 }
 
+static int mtk_thermal_extract_efuse_v2(struct mtk_thermal *mt, u32 *buf)
+{
+   if (!CALIB_BUF1_VALID_V2(buf[1]))
+       return -EINVAL;
+
+   mt->adc_oe = CALIB_BUF0_ADC_OE_V2(buf[0]);
+   mt->adc_ge = CALIB_BUF0_ADC_GE_V2(buf[0]);
+   mt->degc_cali = CALIB_BUF0_DEGC_CALI_V2(buf[0]);
+   mt->o_slope = CALIB_BUF0_O_SLOPE_V2(buf[0]);
+   mt->vts[VTS1] = CALIB_BUF1_VTS_TS1_V2(buf[1]);
+   mt->vts[VTS2] = CALIB_BUF1_VTS_TS2_V2(buf[1]);
+   mt->vts[VTSABB] = CALIB_BUF1_VTS_TSABB_V2(buf[1]);
+   mt->o_slope_sign = CALIB_BUF1_O_SLOPE_SIGN_V2(buf[1]);
+
+   return 0;
+}
+
 static int mtk_thermal_get_calibration_data(struct device *dev,
                        struct mtk_thermal *mt)
 {
@@ -900,6 +966,28 @@ static const struct of_device_id mtk_thermal_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, mtk_thermal_of_match);
 
+static void mtk_thermal_turn_on_buffer(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);
+   udelay(200);
+}
+
+static void mtk_thermal_release_periodic_ts(struct mtk_thermal *mt,
+                       void __iomem *auxadc_base)
+{
+   int tmp;
+
+   writel(0x800, auxadc_base + AUXADC_CON1_SET_V);
+   writel(0x1, mt->thermal_base + TEMP_MONCTL0);
+   tmp = readl(mt->thermal_base + TEMP_MSRCTL1);
+   writel((tmp & (~0x10e)), mt->thermal_base + TEMP_MSRCTL1);
+}
+
 static int mtk_thermal_probe(struct platform_device *pdev)
 {
    int ret, i, ctrl_id;
@@ -908,6 +996,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
    struct resource *res;
    u64 auxadc_phys_base, apmixed_phys_base;
    struct thermal_zone_device *tzdev;
+   void __iomem *apmixed_base, *auxadc_base;
 
    mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
    if (!mt)
@@ -942,6 +1031,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
        return -ENODEV;
    }
 
+   auxadc_base = of_iomap(auxadc, 0);
    auxadc_phys_base = of_get_phys_base(auxadc);
 
    of_node_put(auxadc);
@@ -957,6 +1047,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
        return -ENODEV;
    }
 
+   apmixed_base = of_iomap(apmixedsys, 0);
    apmixed_phys_base = of_get_phys_base(apmixedsys);
 
    of_node_put(apmixedsys);
@@ -982,6 +1073,11 @@ static int mtk_thermal_probe(struct platform_device *pdev)
        goto err_disable_clk_auxadc;
    }
 
+   if (mt->conf->version == MTK_THERMAL_V2) {
+       mtk_thermal_turn_on_buffer(apmixed_base);
+       mtk_thermal_release_periodic_ts(mt, auxadc_base);
+   }
+
    for (ctrl_id = 0; ctrl_id < mt->conf->num_controller ; ctrl_id++)
        for (i = 0; i < mt->conf->num_banks; i++)
            mtk_thermal_init_bank(mt, i, apmixed_phys_base,

Thank you. Have you changed anything else?

Currently don’t want to revert if this is the only change.

I can test on 5.5, but want to leave 5.4 now clean so far :slight_smile: Still wondering about the no calibration data on r2

I don’t change anything else.

Will take some time to look into the no calibration data issue on r2.

Thanks

1 Like

could you rebase to 5.5 final? patches fail and it looks it’s not yet in mainline

currently i have cherry-picked my 5.4 patches to 5.5-main

thanks for your work on the thermal driver… I can confirm, that it works with r64 and kernel 5.4.

The patch below enables hwmon support for the thermal zone from mtk-thermal, so the temperature is visible using the “sensors” command…

Index: linux-5.4.8/drivers/thermal/mtk_thermal.c
===================================================================
--- linux-5.4.8.orig/drivers/thermal/mtk_thermal.c
+++ linux-5.4.8/drivers/thermal/mtk_thermal.c
@@ -23,6 +23,8 @@
 #include <linux/reset.h>
 #include <linux/types.h>
 
+#include "thermal_hwmon.h"
+
 /* AUXADC Registers */
 #define AUXADC_CON1_SET_V      0x008
 #define AUXADC_CON1_CLR_V      0x00c
@@ -990,6 +992,13 @@ static void mtk_thermal_release_periodic
        writel((tmp & (~0x10e)), mt->thermal_base + TEMP_MSRCTL1);
 }
 
+static void mtk_thermal_hwmon_action(void *data)
+{
+       struct thermal_zone_device *zone = data;
+
+       thermal_remove_hwmon_sysfs(zone);
+}
+
 static int mtk_thermal_probe(struct platform_device *pdev)
 {
        int ret, i, ctrl_id;
@@ -1094,6 +1103,17 @@ static int mtk_thermal_probe(struct plat
                goto err_disable_clk_peri_therm;
        }
 
+       tzdev->tzp->no_hwmon = false;
+       ret = thermal_add_hwmon_sysfs(tzdev);
+       if (ret)
+               goto err_disable_clk_peri_therm;
+
+       ret = devm_add_action(&pdev->dev, mtk_thermal_hwmon_action, tzdev);
+       if (ret) {
+               mtk_thermal_hwmon_action(tzdev);
+               goto err_disable_clk_peri_therm;
+       }
+
        return 0;
 
 err_disable_clk_peri_therm:
1 Like

Thank you for adding sensors support. Just a note: shouldn’t it be optional (in case of error do not break probe). I would like only printing message that anything was failing. Is it for r64 only or should it work for all devices using this driver (looks like that)?

indeed, it should also work with other devices… but I have only tested with r64.

Yes, it could make sense to make it more optional… but IMHO a generic approach is needed anyway in order to avoid redundant code (registering, error checking/message printing (driver dependent?) and unloading). There was a patch and discussion here: https://patchwork.kernel.org/patch/9619513/

IMHO such a generic patch is a more reasonable way, if the question was to be solved for more different kernel configurations. But unfortunately, the patch has not been mainlined (yet?)

Jasmin

i wonder about the last line here…i guess ret is set on error…why call mtk_thermal_hwmon_action here? shouldn’t it be run if no error appears?

ok, mtk_thermal_hwmon_action calls thermal_remove_hwmon_sysfs…so it looks right here, but why should this done on action (registered by devm_add_action above)?

the function makes not much sense to me…imho it should read out the value from sysfs, and not disable the hwmon

yes, devm_add_action makes sure that unregistering of hwmon is performed when mtk thermal is unloaded. If it fails to register this “hook” (ret), unregistering hwmon (action) is directly called and thermal driver unloaded (if such a basic operation fails, something severe is not OK: and it is unlikely that, in such a case, one is still interested in using the thermal zone).

The unregistering of hwmon can also be done manually (when unloading mediatek thermal driver), but devm_* (devres) is specifically designed for such cases (generic approach for driver resource management).

Furthermore it maximizes similarity with other thermal drivers (hwmon bridge), so it is easier to convert the thermal-in-hwmon-register-action to a more generic solution (see patch/link/last posting)… :slight_smile: