Re: [PATCH v5 4/5] thermal/drivers/sun8i: Add support for A523 THS0/1 controllers
From: Mikhail Kalashnikov
Date: Sun Sep 06 2026 - 04:57:54 EST
On 7/21/26 23:23, Chen-Yu Tsai wrote:
On Sun, Jul 5, 2026 at 1:15 AM Mikhail Kalashnikov <iuncuim@xxxxxxxxx> wrote:Yes, I think you're right. And as Juan pointed out, my current
The A523 processor has two temperature controllers, THS0 and THS1.
THS0 has only one temperature sensor, which is located in the DRAM
controller. THS1 does have 3 sensors:
ths1_0 - "big" cores
ths1_1 - "little" cores
ths1_2 - gpu
The datasheet mentions a fourth sensor in the NPU, but lacks any registers
for operation other than calibration registers. The vendor code reads the
value from ths1_2, but uses separate calibration data, so we get two
different values from real one.
Are you sure? The calibration data is written to the sensor register.
The BSP simply reads the sensor data from the GPU sensor, and passes it
off as the value for the NPU sensor. No extra calculation involving the
calibration data is done.
implementation doesn't match the BSP, since:
#define SUN55IW3_GPU_SENSOR_ID (2)
#define SUN55IW3_NPU_SENSOR_ID (3)
...
/* use NPU sensor data instead of GPU */
if (id == SUN55IW3_GPU_SENSOR_ID) {
id = SUN55IW3_NPU_SENSOR_ID;
...
}
In other words, in the BSP driver, the NPU sensor is used for the GPU. However, based on the A523 user manual—which describes registers for
only three sensors, I had argued the opposite. The user manual for the
T527 includes a description of a fourth sensor.
Thus, it seems that all four sensors are working, but the purpose of
this workaround in the BSP remains unclear to me.
Signed-off-by: Mikhail Kalashnikov <iuncuim@xxxxxxxxx>
Reviewed-by: Chen-Yu Tsai <wens@xxxxxxxxxx>
---
drivers/thermal/sun8i_thermal.c | 133 ++++++++++++++++++++++++++++++++
1 file changed, 133 insertions(+)
diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
index 3bdd62aa8..f48ed9eae 100644
--- a/drivers/thermal/sun8i_thermal.c
+++ b/drivers/thermal/sun8i_thermal.c
@@ -59,6 +59,12 @@
#define SUN50I_H6_THS_PC_TEMP_PERIOD(x) ((GENMASK(19, 0) & (x)) << 12)
#define SUN50I_H6_THS_DATA_IRQ_STS(x) BIT(x)
+#define SUN55I_A523_DELIMITER 0x7c8
+#define SUN55I_A523_OFFSET_ABOVE 2736
+#define SUN55I_A523_OFFSET_BELOW 2825
+#define SUN55I_A523_SCALE_ABOVE 74
+#define SUN55I_A523_SCALE_BELOW 65
+
struct tsensor {
struct ths_device *tmdev;
struct thermal_zone_device *tzd;
@@ -114,6 +120,15 @@ static int sun50i_h5_calc_temp(struct ths_device *tmdev,
return -1590 * reg / 10 + 276000;
}
+static int sun55i_a523_calc_temp(struct ths_device *tmdev,
+ int id, int reg)
+{
+ if (reg >= SUN55I_A523_DELIMITER)
+ return SUN55I_A523_SCALE_ABOVE * (SUN55I_A523_OFFSET_ABOVE - reg);
+ else
+ return SUN55I_A523_SCALE_BELOW * (SUN55I_A523_OFFSET_BELOW - reg);
+}
+
static int sun8i_ths_get_temp(struct thermal_zone_device *tz, int *temp)
{
struct tsensor *s = thermal_zone_device_priv(tz);
@@ -299,6 +314,97 @@ static int sun50i_h6_ths_calibrate(struct ths_device *tmdev,
return 0;
}
+/*
+ * The A523 nvmem calibration values. The ths1_3 is not used as it
+ * doesn't have its own sensor and doesn't have any internal switch.
+ * Instead, the value from the ths1_2 sensor is used, which gives the
+ * illusion of an independent sensor for NPU and GPU when using
+ * different calibration values.
+ *
+ * efuse layout 0x38-0x3F (caldata[0..3]):
+ * caldata[0] caldata[1] caldata[2] caldata[3]
+ * 0 16 24 32 36 48 60 64
+ * +---------------+---------------+---------------+---------------+
+ * | | | temp | ths1_0 | ths1_1 | +
+ * +---------------+---------------+---------------+---------------+
+ *
+ * efuse layout 0x44-0x4B (caldata[4..7]):
+ * caldata[4] caldata[5] caldata[6] caldata[7]
+ * 0 12 16 24 32 36 48 64
+ * +---------------+---------------+---------------+---------------+
+ * | ths1_2 | ths1_3 | ths0 | | +
+ * +---------------+---------------+---------------+---------------+
+ */
+static int sun55i_a523_ths_calibrate(struct ths_device *tmdev,
+ u16 *caldata, int callen)
+{
+ struct device *dev = tmdev->dev;
+ int i, ft_temp;
+
+ if (!caldata[0])
+ return -EINVAL;
+
+ ft_temp = (((caldata[2] << 8) | (caldata[1] >> 8)) & FT_TEMP_MASK) * 100;
+
+ for (i = 0; i < tmdev->chip->sensor_num; i++) {
+ int sensor_reg, sensor_temp, cdata, offset;
+ /*
+ * Chips ths0 and ths1 have common parameters for value
+ * calibration. To separate them we can use the number of
+ * temperature sensors on each chip.
+ * For ths0 this value is 1.
+ */
+ if (tmdev->chip->sensor_num == 1) {
+ sensor_reg = ((caldata[5] >> 8) | (caldata[6] << 8)) & TEMP_CALIB_MASK;
+ } else {
+ switch (i) {
+ case 0:
+ sensor_reg = (caldata[2] >> 4) & TEMP_CALIB_MASK;
+ break;
+ case 1:
+ sensor_reg = caldata[3] & TEMP_CALIB_MASK;
+ break;
+ case 2:
+ sensor_reg = caldata[4] & TEMP_CALIB_MASK;
+ break;
+ default:
+ sensor_reg = 0;
+ break;
+ }
+ }
+
+ sensor_temp = tmdev->chip->calc_temp(tmdev, i, sensor_reg);
+
+ /*
+ * Calibration data is CALIBRATE_DEFAULT - (calculated
+ * temperature from sensor reading at factory temperature
+ * minus actual factory temperature) * X (scale from
+ * temperature to register values)
+ */
+ cdata = CALIBRATE_DEFAULT -
+ ((sensor_temp - ft_temp) / SUN55I_A523_SCALE_ABOVE);
The BSP version works out to:
cdata = CALIBRATE_DEFAULT -
(sensor_temp - ft_temp - 5000) / SU55I_A523_SCALE_BELOW
Any particular reason why it's different?
In the early versions, I didn't use 'define' for constants, but later
added them based on the logic of the value read from the register: a
value below the threshold is 'BELOW', and a value above is 'ABOVE'. The
BSP driver uses a name for the temperature, but the temperature
decreases as the value read from the register increases; therefore, the
formula uses SCALE constants with different names but identical values.
In the next version, I’ll align them with the BSP values and add a comment explaining that the constants refer to the final temperature value, not the value read from the register.
Regarding SUN55IW3_CAL_COM 5000 — when I first looked at the driver
code, it seemed to me that this was the value intended for ft_deviation,
but now that I’ve double-checked, the version of the calibration formula
with SUN55IW3_CAL_COM and without ft_deviation gives the same result as
the version without SUN55IW3_CAL_COM but with ft_deviation=5000.
But if you'd rather I match BSP literally (5000 in the calibration
numerator, ft_deviation = 0), I can do that instead.
+ if (cdata & ~TEMP_CALIB_MASK) {
+ /*
+ * Calibration value more than 12-bit, but calibration
+ * register is 12-bit. In this case, ths hardware can
+ * still work without calibration, although the data
+ * won't be so accurate.
+ */
+ dev_warn(dev, "sensor%d is not calibrated.\n", i);
+ continue;
+ }
+
+ offset = (i % 2) * 16;
+ regmap_update_bits(tmdev->regmap,
+ SUN50I_H6_THS_TEMP_CALIB + (i / 2 * 4),
+ TEMP_CALIB_MASK << offset,
+ cdata << offset);
+ }
+
+ return 0;
+}
+
static int sun8i_ths_calibrate(struct ths_device *tmdev)
{
struct nvmem_cell *calcell = NULL;
@@ -717,6 +823,31 @@ static const struct ths_thermal_chip sun50i_h616_ths = {
.calc_temp = sun8i_ths_calc_temp,
};
+/* The A523 has a shared reset line for both chips */
+static const struct ths_thermal_chip sun55i_a523_ths0 = {
+ .sensor_num = 1,
+ .has_bus_clk_reset = true,
+ .has_mod_clk = true,
+ .ft_deviation = 5000,
In the BSP this is 0.
+ .temp_data_base = SUN50I_H6_THS_TEMP_DATA,
+ .calibrate = sun55i_a523_ths_calibrate,
+ .init = sun50i_h6_thermal_init,
+ .irq_ack = sun50i_h6_irq_ack,
+ .calc_temp = sun55i_a523_calc_temp,
+};
+
+static const struct ths_thermal_chip sun55i_a523_ths1 = {
+ .sensor_num = 3,
+ .has_bus_clk_reset = true,
+ .has_mod_clk = true,
+ .ft_deviation = 5000,
In the BSP this is 0.
ChenYu
+ .temp_data_base = SUN50I_H6_THS_TEMP_DATA,
+ .calibrate = sun55i_a523_ths_calibrate,
+ .init = sun50i_h6_thermal_init,
+ .irq_ack = sun50i_h6_irq_ack,
+ .calc_temp = sun55i_a523_calc_temp,
+};
+
static const struct of_device_id of_ths_match[] = {
{ .compatible = "allwinner,sun8i-a83t-ths", .data = &sun8i_a83t_ths },
{ .compatible = "allwinner,sun8i-h3-ths", .data = &sun8i_h3_ths },
@@ -727,6 +858,8 @@ static const struct of_device_id of_ths_match[] = {
{ .compatible = "allwinner,sun50i-h6-ths", .data = &sun50i_h6_ths },
{ .compatible = "allwinner,sun20i-d1-ths", .data = &sun20i_d1_ths },
{ .compatible = "allwinner,sun50i-h616-ths", .data = &sun50i_h616_ths },
+ { .compatible = "allwinner,sun55i-a523-ths0", .data = &sun55i_a523_ths0 },
+ { .compatible = "allwinner,sun55i-a523-ths1", .data = &sun55i_a523_ths1 },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, of_ths_match);
--
2.55.0