[2/3] thermal/drivers/sun8i: Add support for the A80 THS
Commit Message
The Allwinner A80 thermal sensor is an early relative of the THS found
in the A83T and later SoCs. It shares its register block, bus gate,
4 MHz module clock and reset line with the GPADC; the THS registers
live at offset 0x40 of the shared block, with four data registers for
the four sensors (0: big cluster, 1: DRAM, 2: GPU, 3: little cluster).
Register layout, initialization values and the temperature formula
(T = 190 - raw * 1000 / 14543, i.e. offset 190000 / scale 688 in this
driver's convention) are taken from the vendor BSP kernel. The init
sequence also programs the hardware alarm (~90 degC) and emergency
shutdown (~105 degC) thresholds the BSP uses, since their reset
defaults are undefined.
Unlike on later SoCs the data-ready interrupt fires at the conversion
rate (tens of kHz), so it is left disabled and the thermal core polls
the sensors; the interrupt line only serves the alarm and shutdown
events, matching the vendor BSP behaviour.
The A80 SID is not yet supported by the sunxi nvmem driver, so
calibration data is currently unavailable and the driver falls back to
the defaults, which yield plausible results (idle temperatures around
40 degC, full-load peaks in the mid 50s on a Cubieboard4).
Tested on a Cubietech Cubieboard4: all four zones report load-reactive
temperatures via polling.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Sören Hantel <fugininsane@googlemail.com>
---
drivers/thermal/sun8i_thermal.c | 93 ++++++++++++++++++++++++++++++++-
1 file changed, 92 insertions(+), 1 deletion(-)
Comments
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Unconditional NULL pointer dereference if calibration nvmem cell is present.
- [High] Statically enabled ALARM interrupt will cause a severe interrupt storm when triggered.
Pre-existing issues:
- [High] Interrupt handler defeats spurious interrupt detection by unconditionally returning IRQ_HANDLED.
--
--- Patch [2]: [PATCH 2/3] thermal/drivers/sun8i: Add support for the A80 THS ---
commit 95ccac1aebf565b0761d7e8e612500acbb05beba
Author: Sören Hantel <fugininsane@gmail.com>
thermal/drivers/sun8i: Add support for the A80 THS
The Allwinner A80 thermal sensor is an early relative of the THS found
in the A83T and later SoCs. It shares its register block, bus gate,
4 MHz module clock and reset line with the GPADC; the THS registers
live at offset 0x40 of the shared block, with four data registers for
the four sensors (0: big cluster, 1: DRAM, 2: GPU, 3: little cluster).
> diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
> index 22674790629a7..3e7bfda621267 100644
> --- a/drivers/thermal/sun8i_thermal.c
> +++ b/drivers/thermal/sun8i_thermal.c
[ ... ]
> @@ -596,6 +616,76 @@ static int sun8i_ths_probe(struct platform_device *pdev)
> return 0;
> }
>
> +static int sun9i_a80_thermal_init(struct ths_device *tmdev)
> +{
> + int i;
> +
> + /* clear any pending interrupt status */
> + regmap_write(tmdev->regmap, SUN9I_THS_IS, 0xfff);
> + /* set up the median filter, average over 8 samples */
> + regmap_write(tmdev->regmap, SUN9I_THS_MFC, 0x5);
> +
> + /*
> + * Program the protection thresholds with the values the vendor
> + * BSP uses (thresholds are in raw sensor units, which decrease
> + * with rising temperature): an alarm interrupt at ~90 degC and
> + * an emergency hardware shutdown at ~105 degC.
> + */
> + for (i = 0; i < tmdev->chip->sensor_num; i++) {
> + regmap_write(tmdev->regmap, SUN9I_THS_ALARM_TH(i),
> + (1454 << 16) | 0xfff);
> + regmap_write(tmdev->regmap, SUN9I_THS_SHUT_TH(i),
> + (1231 << 16) | 0xfff);
> + }
> +
> + /*
> + * Unlike on later SoCs, the data-ready interrupt fires at the
> + * conversion rate (tens of kHz), so leave it disabled and let the
> + * thermal core poll the data registers; the interrupt line only
> + * serves the alarm and shutdown events, like in the vendor BSP.
> + */
> + regmap_write(tmdev->regmap, SUN9I_THS_IC,
> + SUN9I_THS_ALARM_IRQ_EN | SUN9I_THS_SHUT_IRQ_EN);
[Severity: High]
Will this statically enabled ALARM interrupt cause an interrupt storm when
triggered?
Since the ALARM threshold is static (~90 degC) and sun8i_irq_thread() clears
the status bit but does not mask the interrupt or adjust the threshold (as
there is no set_trips callback in this driver), won't the hardware immediately
re-assert the interrupt once the IRQF_ONESHOT thread unmasks the line while
the temperature remains above the threshold?
> + /* acquire time 0x2f, enable all four sensors */
> + regmap_write(tmdev->regmap, SUN9I_THS_CTRL,
> + SUN9I_THS_CTRL_ACQ(0x2f) | SUN9I_THS_CTRL_SENSOR_EN);
> +
> + return 0;
> +}
[ ... ]
> +static const struct ths_thermal_chip sun9i_a80_ths = {
> + .sensor_num = 4,
> + .has_mod_clk = true,
> + .mod_clk_rate = 4000000,
> + .has_bus_clk_reset = true,
> + .scale = 688,
> + .offset = 190000,
> + .temp_data_base = SUN9I_THS_TEMP_DATA,
> + .init = sun9i_a80_thermal_init,
[Severity: High]
Is it possible for the calibration nvmem cell to be present for this chip?
The sun8i_ths_calibrate() function unconditionally dereferences the .calibrate
callback if calibration data is successfully read:
drivers/thermal/sun8i_thermal.c:sun8i_ths_calibrate() {
...
caldata = nvmem_cell_read(calcell, &callen);
if (IS_ERR(caldata)) {
ret = PTR_ERR(caldata);
goto out;
}
tmdev->chip->calibrate(tmdev, caldata, callen);
...
}
Since the sun9i_a80_ths struct omits the .calibrate callback, if a calibration
nvmem cell is provided (e.g. via a device tree overlay), would this cause a
NULL pointer dereference during probe?
> + .irq_ack = sun9i_a80_irq_ack,
[Severity: High]
This is a pre-existing issue, but does sun8i_irq_thread() defeat spurious
interrupt detection by unconditionally returning IRQ_HANDLED?
If the hardware triggers an interrupt but irq_ack() finds no relevant status
bits (returning an irq_bitmap of 0), the loop is skipped:
drivers/thermal/sun8i_thermal.c:sun8i_irq_thread() {
struct ths_device *tmdev = data;
unsigned long irq_bitmap = tmdev->chip->irq_ack(tmdev);
int i;
for_each_set_bit(i, &irq_bitmap, tmdev->chip->sensor_num) {
...
}
return IRQ_HANDLED;
}
Returning IRQ_HANDLED instead of IRQ_NONE when no status bits are set prevents
the core IRQ subsystem from detecting and disabling a stuck or spurious
interrupt line.
> + .calc_temp = sun8i_ths_calc_temp,
> +};
> +
> static const struct ths_thermal_chip sun8i_a83t_ths = {
> .sensor_num = 3,
> .scale = 705,
@@ -65,8 +65,27 @@ struct tsensor {
int id;
};
+/*
+ * The A80 thermal sensor shares its register block with the GPADC.
+ * The THS registers start at offset 0x40 within that block.
+ */
+#define SUN9I_THS_CTRL 0x40
+#define SUN9I_THS_IC 0x44
+#define SUN9I_THS_IS 0x48
+#define SUN9I_THS_ALARM_TH(x) (0x50 + (x) * 0x4)
+#define SUN9I_THS_SHUT_TH(x) (0x60 + (x) * 0x4)
+#define SUN9I_THS_MFC 0x70
+#define SUN9I_THS_TEMP_DATA 0x80
+#define SUN9I_THS_CTRL_ACQ(x) ((x) << 16)
+#define SUN9I_THS_CTRL_SENSOR_EN GENMASK(3, 0)
+#define SUN9I_THS_ALARM_IRQ_EN GENMASK(3, 0)
+#define SUN9I_THS_SHUT_IRQ_EN GENMASK(7, 4)
+#define SUN9I_THS_ALARM_IRQ_STS(x) BIT(x)
+#define SUN9I_THS_SHUT_IRQ_STS(x) BIT(4 + (x))
+
struct ths_thermal_chip {
bool has_mod_clk;
+ unsigned long mod_clk_rate;
bool has_bus_clk_reset;
bool needs_sram;
int sensor_num;
@@ -413,7 +432,8 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev)
return PTR_ERR(tmdev->mod_clk);
}
- ret = clk_set_rate(tmdev->mod_clk, 24000000);
+ ret = clk_set_rate(tmdev->mod_clk, tmdev->chip->mod_clk_rate ?:
+ 24000000);
if (ret)
return ret;
@@ -596,6 +616,76 @@ static int sun8i_ths_probe(struct platform_device *pdev)
return 0;
}
+static int sun9i_a80_thermal_init(struct ths_device *tmdev)
+{
+ int i;
+
+ /* clear any pending interrupt status */
+ regmap_write(tmdev->regmap, SUN9I_THS_IS, 0xfff);
+ /* set up the median filter, average over 8 samples */
+ regmap_write(tmdev->regmap, SUN9I_THS_MFC, 0x5);
+
+ /*
+ * Program the protection thresholds with the values the vendor
+ * BSP uses (thresholds are in raw sensor units, which decrease
+ * with rising temperature): an alarm interrupt at ~90 degC and
+ * an emergency hardware shutdown at ~105 degC.
+ */
+ for (i = 0; i < tmdev->chip->sensor_num; i++) {
+ regmap_write(tmdev->regmap, SUN9I_THS_ALARM_TH(i),
+ (1454 << 16) | 0xfff);
+ regmap_write(tmdev->regmap, SUN9I_THS_SHUT_TH(i),
+ (1231 << 16) | 0xfff);
+ }
+
+ /*
+ * Unlike on later SoCs, the data-ready interrupt fires at the
+ * conversion rate (tens of kHz), so leave it disabled and let the
+ * thermal core poll the data registers; the interrupt line only
+ * serves the alarm and shutdown events, like in the vendor BSP.
+ */
+ regmap_write(tmdev->regmap, SUN9I_THS_IC,
+ SUN9I_THS_ALARM_IRQ_EN | SUN9I_THS_SHUT_IRQ_EN);
+ /* acquire time 0x2f, enable all four sensors */
+ regmap_write(tmdev->regmap, SUN9I_THS_CTRL,
+ SUN9I_THS_CTRL_ACQ(0x2f) | SUN9I_THS_CTRL_SENSOR_EN);
+
+ return 0;
+}
+
+static unsigned long sun9i_a80_irq_ack(struct ths_device *tmdev)
+{
+ unsigned long irq_bitmap = 0;
+ int i, state;
+
+ regmap_read(tmdev->regmap, SUN9I_THS_IS, &state);
+
+ for (i = 0; i < MAX_SENSOR_NUM; i++) {
+ if (state & (SUN9I_THS_ALARM_IRQ_STS(i) |
+ SUN9I_THS_SHUT_IRQ_STS(i))) {
+ regmap_write(tmdev->regmap, SUN9I_THS_IS,
+ state & (SUN9I_THS_ALARM_IRQ_STS(i) |
+ SUN9I_THS_SHUT_IRQ_STS(i)));
+ set_bit(i, &irq_bitmap);
+ }
+ }
+
+ return irq_bitmap;
+}
+
+static const struct ths_thermal_chip sun9i_a80_ths = {
+ .sensor_num = 4,
+ .has_mod_clk = true,
+ .mod_clk_rate = 4000000,
+ .has_bus_clk_reset = true,
+ .scale = 688,
+ .offset = 190000,
+ .temp_data_base = SUN9I_THS_TEMP_DATA,
+ .init = sun9i_a80_thermal_init,
+ .irq_ack = sun9i_a80_irq_ack,
+ .calc_temp = sun8i_ths_calc_temp,
+};
+
static const struct ths_thermal_chip sun8i_a83t_ths = {
.sensor_num = 3,
.scale = 705,
@@ -711,6 +801,7 @@ static const struct ths_thermal_chip sun50i_h616_ths = {
static const struct of_device_id of_ths_match[] = {
{ .compatible = "allwinner,sun8i-a83t-ths", .data = &sun8i_a83t_ths },
+ { .compatible = "allwinner,sun9i-a80-ths", .data = &sun9i_a80_ths },
{ .compatible = "allwinner,sun8i-h3-ths", .data = &sun8i_h3_ths },
{ .compatible = "allwinner,sun8i-r40-ths", .data = &sun8i_r40_ths },
{ .compatible = "allwinner,sun50i-a64-ths", .data = &sun50i_a64_ths },