[v5,4/5] thermal/drivers/sun8i: Add support for A523 THS0/1 controllers

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.

Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com>
Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
---
 drivers/thermal/sun8i_thermal.c | 133 ++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)
  

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The calibration check `if (!caldata[0])` will always fail and abort calibration if bits 0-15 of the A523 eFuse are zero.
- [High] NULL pointer dereference if the `nvmem-cell-names` property is missing from the Device Tree.
- [Medium] Out-of-bounds heap read due to missing `callen` validation in `sun55i_a523_ths_calibrate`.
- [Medium] Calibration logic incorrectly hardcodes `SUN55I_A523_SCALE_ABOVE` to convert temperature error into a register offset.
--

commit ed04dff3819d46435d0d0c6fa95dfd782738eb1e
Author: Mikhail Kalashnikov <iuncuim@gmail.com>

thermal/drivers/sun8i: Add support for A523 THS0/1 controllers

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.

> diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
> index 3bdd62aa8bdf1..f48ed9eae6c4f 100644
> --- a/drivers/thermal/sun8i_thermal.c
> +++ b/drivers/thermal/sun8i_thermal.c

[ ... ]

> @@ -299,6 +314,97 @@ static int sun50i_h6_ths_calibrate(struct ths_device *tmdev,

[ ... ]

> +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])

[Severity: High]
If the device tree lacks the nvmem-cell-names property, the loop inside
sun8i_ths_calibrate() will be skipped entirely. This leaves caldata as
NULL when it is passed to the chip-specific calibrate callback:

sun8i_ths_calibrate() {
    ...
    of_property_for_each_string(...) {
        ...
    }
    tmdev->chip->calibrate(tmdev, (u16 *)caldata, callen);
    ...
}

Will checking !caldata[0] here cause a NULL pointer dereference in that
scenario?

[Severity: High]
According to the eFuse layout diagram in the comment block just above this
function, bits 0-15 are empty/unused, while the actual factory temperature
is stored in bits 24-35 (caldata[1] and caldata[2]).

If those unused eFuse bits are typically zero, will this check silently
abort the calibration process on the A523?

> +		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;

[Severity: Medium]
Is there a chance we might read out of bounds here if the provided nvmem
data is smaller than expected?

If the device tree or nvmem cell provides fewer bytes than the 14 bytes
required to access caldata[6], this could result in an out-of-bounds heap
read.

Should we validate callen before accessing these offsets?

> +		} else {
> +			switch (i) {
> +			case 0:
> +				sensor_reg = (caldata[2] >> 4) & TEMP_CALIB_MASK;
> +				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);

[Severity: Medium]
The sun55i_a523_calc_temp() function uses a piecewise formula depending
on the register value, applying either SUN55I_A523_SCALE_ABOVE or
SUN55I_A523_SCALE_BELOW.

However, the calibration offset calculation here unconditionally divides
the temperature error by SUN55I_A523_SCALE_ABOVE.

If the factory temperature corresponds to a register value in the
SUN55I_A523_SCALE_BELOW region, will dividing by the SCALE_ABOVE
value result in an inaccurate calibration offset?

> +
> +		if (cdata & ~TEMP_CALIB_MASK) {
> +			/*
> +			 * Calibration value more than 12-bit, but calibration
> +			 * register is 12-bit. In this case, ths hardware can
  

On Sun, Jul 5, 2026 at 1:15 AM Mikhail Kalashnikov <iuncuim@gmail.com> wrote:
>
> 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.

>
> Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com>
> Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
> ---
>  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?

> +               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
>