* [PATCH 2/2] thermal: amlogic: Add support for A9 thermal controller
@ 2026-07-30 8:34 ` Xianwei Zhao via B4 Relay
0 siblings, 0 replies; 15+ messages in thread
From: Xianwei Zhao @ 2026-07-30 8:34 UTC (permalink / raw)
To: Guillaume La Roque, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
Lukasz Luba, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-amlogic, devicetree, linux-kernel, Xianwei Zhao
Add support for the Amlogic A9 thermal controller.
The A9 thermal controller uses different calibration parameters, requires
separate core and bus clocks, and supports a hardware over-temperature
reset mechanism.
Add the A9 calibration data, retrieve the clocks by name, and configure
the hardware reset temperature through the optional 'amlogic,hw-reset-temp'
DT property. When the property is absent, use the default reset temperature
of 110°C.
Also restore the hardware reset configuration after resume.
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
drivers/thermal/amlogic_thermal.c | 107 ++++++++++++++++++++++++++++++++++++--
1 file changed, 104 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
index a0b530624b60..c9c351496653 100644
--- a/drivers/thermal/amlogic_thermal.c
+++ b/drivers/thermal/amlogic_thermal.c
@@ -45,6 +45,18 @@
TSENSOR_CFG_REG1_DEM_EN | \
TSENSOR_CFG_REG1_CH_SEL)
+#define TSENSOR_CFG_REG2 0x8
+ #define TSENSOR_CFG_REG2_HITEMP_EN BIT(31)
+ #define TSENSOR_CFG_REG2_REBOOT_EN BIT(30)
+ #define TSENSOR_CFG_REG2_REBOOT_CODE GENMASK(15, 4)
+ #define TSENSOR_CFG_REG2_REBOOT_TIME GENMASK(23, 16)
+ #define TSENSOR_CFG_REG2_ENABLE \
+ (TSENSOR_CFG_REG2_HITEMP_EN | \
+ TSENSOR_CFG_REG2_REBOOT_EN | \
+ TSENSOR_CFG_REG2_REBOOT_TIME)
+
+#define TSENSOR_TEMP_CAL 1
+
#define TSENSOR_STAT0 0x40
#define TSENSOR_STAT9 0x64
@@ -63,6 +75,7 @@
#define TSENSOR_CALIB_OFFSET 1
#define TSENSOR_CALIB_SHIFT 4
+#define TSENSOR_HW_RESET_DEFAULT_TEMP 110000
/**
* struct amlogic_thermal_soc_calib_data
@@ -93,6 +106,7 @@ struct amlogic_thermal_data {
const struct amlogic_thermal_soc_calib_data *calibration_parameters;
const struct regmap_config *regmap_config;
bool use_sm;
+ bool has_sysclk;
};
struct amlogic_thermal {
@@ -101,8 +115,10 @@ struct amlogic_thermal {
struct regmap *regmap;
struct regmap *sec_ao_map;
struct clk *clk;
+ struct clk *sysclk;
struct thermal_zone_device *tzd;
u32 trim_info;
+ u32 temp_code;
struct meson_sm_firmware *sm_fw;
u32 tsensor_id;
};
@@ -138,6 +154,46 @@ static int amlogic_thermal_code_to_millicelsius(struct amlogic_thermal *pdata,
return temp;
}
+/*
+ * Calculate a temperature code from a temperature value .
+ * The unit of the temperature is degree milliCelsius.
+ */
+static u32 amlogic_thermal_millicelsius_to_code(struct amlogic_thermal *pdata, int millicelsius)
+{
+ const struct amlogic_thermal_soc_calib_data *param =
+ pdata->data->calibration_parameters;
+ s64 factor, uptat, uefuse;
+ u32 temp_code;
+
+ uefuse = pdata->trim_info & TSENSOR_TRIM_SIGN_MASK ?
+ ~(pdata->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 :
+ (pdata->trim_info & TSENSOR_TRIM_TEMP_MASK);
+
+ factor = param->B + div_s64(millicelsius, 100);
+ factor = BIT(16) * factor;
+ factor = div_s64(factor, param->A);
+ factor = factor - (uefuse & TSENSOR_TRIM_TEMP_MASK);
+
+ uptat = param->n * factor;
+ uptat = div_s64(uptat, BIT(16));
+ uptat = param->m - uptat;
+
+ factor = factor * 100;
+ factor = div_s64(factor, uptat);
+
+ temp_code = ((factor >> 0x4) & TSENSOR_TEMP_MASK) + TSENSOR_TEMP_CAL;
+
+ return temp_code;
+}
+
+static void amlogic_tsensor_setup_hw_reset(struct amlogic_thermal *data)
+{
+ regmap_update_bits(data->regmap, TSENSOR_CFG_REG2, TSENSOR_CFG_REG2_REBOOT_CODE,
+ data->temp_code << 0x4);
+ regmap_update_bits(data->regmap, TSENSOR_CFG_REG2,
+ TSENSOR_CFG_REG2_ENABLE, TSENSOR_CFG_REG2_ENABLE);
+}
+
static int amlogic_thermal_enable(struct amlogic_thermal *data)
{
int ret;
@@ -254,6 +310,13 @@ static const struct amlogic_thermal_soc_calib_data amlogic_thermal_g12a = {
.n = 324,
};
+static const struct amlogic_thermal_soc_calib_data amlogic_thermal_a9 = {
+ .A = 9164,
+ .B = 2747,
+ .m = 370,
+ .n = 270,
+};
+
static const struct amlogic_thermal_data amlogic_thermal_g12a_cpu_param = {
.u_efuse_off = 0x128,
.calibration_parameters = &amlogic_thermal_g12a,
@@ -272,6 +335,13 @@ static const struct amlogic_thermal_data amlogic_thermal_a1_cpu_param = {
.regmap_config = &amlogic_thermal_regmap_config_g12a,
};
+static const struct amlogic_thermal_data amlogic_thermal_a9_param = {
+ .use_sm = true,
+ .has_sysclk = true,
+ .calibration_parameters = &amlogic_thermal_a9,
+ .regmap_config = &amlogic_thermal_regmap_config_g12a,
+};
+
static const struct amlogic_thermal_data amlogic_thermal_t7_param = {
.use_sm = true,
.calibration_parameters = &amlogic_thermal_g12a,
@@ -291,6 +361,10 @@ static const struct of_device_id of_amlogic_thermal_match[] = {
.compatible = "amlogic,a1-cpu-thermal",
.data = &amlogic_thermal_a1_cpu_param,
},
+ {
+ .compatible = "amlogic,a9-thermal",
+ .data = &amlogic_thermal_a9_param,
+ },
{
.compatible = "amlogic,t7-thermal",
.data = &amlogic_thermal_t7_param,
@@ -305,6 +379,7 @@ static int amlogic_thermal_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
void __iomem *base;
int ret;
+ u32 reset_temp;
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
@@ -323,9 +398,24 @@ static int amlogic_thermal_probe(struct platform_device *pdev)
if (IS_ERR(pdata->regmap))
return PTR_ERR(pdata->regmap);
- pdata->clk = devm_clk_get(dev, NULL);
- if (IS_ERR(pdata->clk))
- return dev_err_probe(dev, PTR_ERR(pdata->clk), "failed to get clock\n");
+ if (pdata->data->has_sysclk) {
+ pdata->clk = devm_clk_get(dev, "core");
+ if (IS_ERR(pdata->clk))
+ return dev_err_probe(dev, PTR_ERR(pdata->clk), "failed to get core clk\n");
+ pdata->sysclk = devm_clk_get_enabled(dev, "pclk");
+ if (IS_ERR(pdata->sysclk))
+ return dev_err_probe(dev, PTR_ERR(pdata->sysclk), "failed to get pclk\n");
+ } else {
+ pdata->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(pdata->clk))
+ return dev_err_probe(dev, PTR_ERR(pdata->clk), "failed to get clock\n");
+ }
+
+ if (of_property_read_u32(pdev->dev.of_node, "amlogic,hw-reset-temp", &reset_temp)) {
+ dev_dbg(dev, "using default hardware reset temperature\n");
+ reset_temp = TSENSOR_HW_RESET_DEFAULT_TEMP;
+ }
+
if (pdata->data->use_sm)
ret = amlogic_thermal_probe_sm(pdev, pdata);
@@ -346,6 +436,10 @@ static int amlogic_thermal_probe(struct platform_device *pdev)
devm_thermal_add_hwmon_sysfs(&pdev->dev, pdata->tzd);
+ pdata->temp_code = amlogic_thermal_millicelsius_to_code(pdata, reset_temp);
+
+ amlogic_tsensor_setup_hw_reset(pdata);
+
ret = amlogic_thermal_enable(pdata);
return ret;
@@ -355,6 +449,8 @@ static void amlogic_thermal_remove(struct platform_device *pdev)
{
struct amlogic_thermal *data = platform_get_drvdata(pdev);
+ if (data->data->has_sysclk)
+ clk_disable_unprepare(data->sysclk);
amlogic_thermal_disable(data);
}
@@ -363,6 +459,8 @@ static int amlogic_thermal_suspend(struct device *dev)
struct amlogic_thermal *data = dev_get_drvdata(dev);
amlogic_thermal_disable(data);
+ if (data->data->has_sysclk)
+ clk_disable_unprepare(data->sysclk);
return 0;
}
@@ -371,6 +469,9 @@ static int amlogic_thermal_resume(struct device *dev)
{
struct amlogic_thermal *data = dev_get_drvdata(dev);
+ amlogic_tsensor_setup_hw_reset(data);
+ if (data->data->has_sysclk)
+ clk_prepare_enable(data->sysclk);
return amlogic_thermal_enable(data);
}
--
2.52.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/2] thermal: amlogic: Add support for A9 thermal controller
@ 2026-07-30 8:34 ` Xianwei Zhao via B4 Relay
0 siblings, 0 replies; 15+ messages in thread
From: Xianwei Zhao via B4 Relay @ 2026-07-30 8:34 UTC (permalink / raw)
To: Guillaume La Roque, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
Lukasz Luba, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-amlogic, devicetree, linux-kernel, Xianwei Zhao
From: Xianwei Zhao <xianwei.zhao@amlogic.com>
Add support for the Amlogic A9 thermal controller.
The A9 thermal controller uses different calibration parameters, requires
separate core and bus clocks, and supports a hardware over-temperature
reset mechanism.
Add the A9 calibration data, retrieve the clocks by name, and configure
the hardware reset temperature through the optional 'amlogic,hw-reset-temp'
DT property. When the property is absent, use the default reset temperature
of 110°C.
Also restore the hardware reset configuration after resume.
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
drivers/thermal/amlogic_thermal.c | 107 ++++++++++++++++++++++++++++++++++++--
1 file changed, 104 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
index a0b530624b60..c9c351496653 100644
--- a/drivers/thermal/amlogic_thermal.c
+++ b/drivers/thermal/amlogic_thermal.c
@@ -45,6 +45,18 @@
TSENSOR_CFG_REG1_DEM_EN | \
TSENSOR_CFG_REG1_CH_SEL)
+#define TSENSOR_CFG_REG2 0x8
+ #define TSENSOR_CFG_REG2_HITEMP_EN BIT(31)
+ #define TSENSOR_CFG_REG2_REBOOT_EN BIT(30)
+ #define TSENSOR_CFG_REG2_REBOOT_CODE GENMASK(15, 4)
+ #define TSENSOR_CFG_REG2_REBOOT_TIME GENMASK(23, 16)
+ #define TSENSOR_CFG_REG2_ENABLE \
+ (TSENSOR_CFG_REG2_HITEMP_EN | \
+ TSENSOR_CFG_REG2_REBOOT_EN | \
+ TSENSOR_CFG_REG2_REBOOT_TIME)
+
+#define TSENSOR_TEMP_CAL 1
+
#define TSENSOR_STAT0 0x40
#define TSENSOR_STAT9 0x64
@@ -63,6 +75,7 @@
#define TSENSOR_CALIB_OFFSET 1
#define TSENSOR_CALIB_SHIFT 4
+#define TSENSOR_HW_RESET_DEFAULT_TEMP 110000
/**
* struct amlogic_thermal_soc_calib_data
@@ -93,6 +106,7 @@ struct amlogic_thermal_data {
const struct amlogic_thermal_soc_calib_data *calibration_parameters;
const struct regmap_config *regmap_config;
bool use_sm;
+ bool has_sysclk;
};
struct amlogic_thermal {
@@ -101,8 +115,10 @@ struct amlogic_thermal {
struct regmap *regmap;
struct regmap *sec_ao_map;
struct clk *clk;
+ struct clk *sysclk;
struct thermal_zone_device *tzd;
u32 trim_info;
+ u32 temp_code;
struct meson_sm_firmware *sm_fw;
u32 tsensor_id;
};
@@ -138,6 +154,46 @@ static int amlogic_thermal_code_to_millicelsius(struct amlogic_thermal *pdata,
return temp;
}
+/*
+ * Calculate a temperature code from a temperature value .
+ * The unit of the temperature is degree milliCelsius.
+ */
+static u32 amlogic_thermal_millicelsius_to_code(struct amlogic_thermal *pdata, int millicelsius)
+{
+ const struct amlogic_thermal_soc_calib_data *param =
+ pdata->data->calibration_parameters;
+ s64 factor, uptat, uefuse;
+ u32 temp_code;
+
+ uefuse = pdata->trim_info & TSENSOR_TRIM_SIGN_MASK ?
+ ~(pdata->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 :
+ (pdata->trim_info & TSENSOR_TRIM_TEMP_MASK);
+
+ factor = param->B + div_s64(millicelsius, 100);
+ factor = BIT(16) * factor;
+ factor = div_s64(factor, param->A);
+ factor = factor - (uefuse & TSENSOR_TRIM_TEMP_MASK);
+
+ uptat = param->n * factor;
+ uptat = div_s64(uptat, BIT(16));
+ uptat = param->m - uptat;
+
+ factor = factor * 100;
+ factor = div_s64(factor, uptat);
+
+ temp_code = ((factor >> 0x4) & TSENSOR_TEMP_MASK) + TSENSOR_TEMP_CAL;
+
+ return temp_code;
+}
+
+static void amlogic_tsensor_setup_hw_reset(struct amlogic_thermal *data)
+{
+ regmap_update_bits(data->regmap, TSENSOR_CFG_REG2, TSENSOR_CFG_REG2_REBOOT_CODE,
+ data->temp_code << 0x4);
+ regmap_update_bits(data->regmap, TSENSOR_CFG_REG2,
+ TSENSOR_CFG_REG2_ENABLE, TSENSOR_CFG_REG2_ENABLE);
+}
+
static int amlogic_thermal_enable(struct amlogic_thermal *data)
{
int ret;
@@ -254,6 +310,13 @@ static const struct amlogic_thermal_soc_calib_data amlogic_thermal_g12a = {
.n = 324,
};
+static const struct amlogic_thermal_soc_calib_data amlogic_thermal_a9 = {
+ .A = 9164,
+ .B = 2747,
+ .m = 370,
+ .n = 270,
+};
+
static const struct amlogic_thermal_data amlogic_thermal_g12a_cpu_param = {
.u_efuse_off = 0x128,
.calibration_parameters = &amlogic_thermal_g12a,
@@ -272,6 +335,13 @@ static const struct amlogic_thermal_data amlogic_thermal_a1_cpu_param = {
.regmap_config = &amlogic_thermal_regmap_config_g12a,
};
+static const struct amlogic_thermal_data amlogic_thermal_a9_param = {
+ .use_sm = true,
+ .has_sysclk = true,
+ .calibration_parameters = &amlogic_thermal_a9,
+ .regmap_config = &amlogic_thermal_regmap_config_g12a,
+};
+
static const struct amlogic_thermal_data amlogic_thermal_t7_param = {
.use_sm = true,
.calibration_parameters = &amlogic_thermal_g12a,
@@ -291,6 +361,10 @@ static const struct of_device_id of_amlogic_thermal_match[] = {
.compatible = "amlogic,a1-cpu-thermal",
.data = &amlogic_thermal_a1_cpu_param,
},
+ {
+ .compatible = "amlogic,a9-thermal",
+ .data = &amlogic_thermal_a9_param,
+ },
{
.compatible = "amlogic,t7-thermal",
.data = &amlogic_thermal_t7_param,
@@ -305,6 +379,7 @@ static int amlogic_thermal_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
void __iomem *base;
int ret;
+ u32 reset_temp;
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
@@ -323,9 +398,24 @@ static int amlogic_thermal_probe(struct platform_device *pdev)
if (IS_ERR(pdata->regmap))
return PTR_ERR(pdata->regmap);
- pdata->clk = devm_clk_get(dev, NULL);
- if (IS_ERR(pdata->clk))
- return dev_err_probe(dev, PTR_ERR(pdata->clk), "failed to get clock\n");
+ if (pdata->data->has_sysclk) {
+ pdata->clk = devm_clk_get(dev, "core");
+ if (IS_ERR(pdata->clk))
+ return dev_err_probe(dev, PTR_ERR(pdata->clk), "failed to get core clk\n");
+ pdata->sysclk = devm_clk_get_enabled(dev, "pclk");
+ if (IS_ERR(pdata->sysclk))
+ return dev_err_probe(dev, PTR_ERR(pdata->sysclk), "failed to get pclk\n");
+ } else {
+ pdata->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(pdata->clk))
+ return dev_err_probe(dev, PTR_ERR(pdata->clk), "failed to get clock\n");
+ }
+
+ if (of_property_read_u32(pdev->dev.of_node, "amlogic,hw-reset-temp", &reset_temp)) {
+ dev_dbg(dev, "using default hardware reset temperature\n");
+ reset_temp = TSENSOR_HW_RESET_DEFAULT_TEMP;
+ }
+
if (pdata->data->use_sm)
ret = amlogic_thermal_probe_sm(pdev, pdata);
@@ -346,6 +436,10 @@ static int amlogic_thermal_probe(struct platform_device *pdev)
devm_thermal_add_hwmon_sysfs(&pdev->dev, pdata->tzd);
+ pdata->temp_code = amlogic_thermal_millicelsius_to_code(pdata, reset_temp);
+
+ amlogic_tsensor_setup_hw_reset(pdata);
+
ret = amlogic_thermal_enable(pdata);
return ret;
@@ -355,6 +449,8 @@ static void amlogic_thermal_remove(struct platform_device *pdev)
{
struct amlogic_thermal *data = platform_get_drvdata(pdev);
+ if (data->data->has_sysclk)
+ clk_disable_unprepare(data->sysclk);
amlogic_thermal_disable(data);
}
@@ -363,6 +459,8 @@ static int amlogic_thermal_suspend(struct device *dev)
struct amlogic_thermal *data = dev_get_drvdata(dev);
amlogic_thermal_disable(data);
+ if (data->data->has_sysclk)
+ clk_disable_unprepare(data->sysclk);
return 0;
}
@@ -371,6 +469,9 @@ static int amlogic_thermal_resume(struct device *dev)
{
struct amlogic_thermal *data = dev_get_drvdata(dev);
+ amlogic_tsensor_setup_hw_reset(data);
+ if (data->data->has_sysclk)
+ clk_prepare_enable(data->sysclk);
return amlogic_thermal_enable(data);
}
--
2.52.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/2] thermal: amlogic: Add support for A9 thermal controller
2026-07-30 8:34 ` Xianwei Zhao via B4 Relay
@ 2026-07-30 8:46 ` sashiko-bot
-1 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-07-30 8:46 UTC (permalink / raw)
To: Xianwei Zhao; +Cc: linux-amlogic, neil.armstrong, robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
New issues:
- [High] Double disable of 'sysclk' due to manual disable of devm-managed clock in remove callback.
- [High] Peripheral hardware registers are accessed via MMIO before their backing clocks are enabled.
- [High] Incorrect hardware reset temperature calculation for negative calibration trims due to flawed bitwise masking.
- [High] A9-specific hardware over-temperature reset configuration is applied unconditionally to all legacy SoCs.
- [High] The return value of `clk_prepare_enable(data->sysclk)` is ignored in the resume path.
- [High] An unvalidated device tree property can cause a divide-by-zero kernel panic during probe.
- [Medium] Possible truncation of `temp_code` during hardware reset configuration due to out-of-bounds bitwise masking.
Pre-existing issues:
- [High] The thermal zone is registered and unregistered in an unsafe order relative to hardware clock state.
--
--- Patch [2]: [PATCH 2/2] thermal: amlogic: Add support for A9 thermal controller ---
commit 0d417d06a989692f70ed92067eec1c72408bd3da
Author: Xianwei Zhao <xianwei.zhao@amlogic.com>
thermal: amlogic: Add support for A9 thermal controller
Add support for the Amlogic A9 thermal controller, utilizing different
calibration parameters, separate core and bus clocks, and a hardware
over-temperature reset mechanism. The hardware reset configuration is
also restored after resume.
> diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
> index a0b530624b60c..c9c3514966533 100644
> --- a/drivers/thermal/amlogic_thermal.c
> +++ b/drivers/thermal/amlogic_thermal.c
[ ... ]
> @@ -138,6 +154,46 @@ static int amlogic_thermal_code_to_millicelsius(struct amlogic_thermal *pdata,
> return temp;
> }
>
> +/*
> + * Calculate a temperature code from a temperature value .
> + * The unit of the temperature is degree milliCelsius.
> + */
> +static u32 amlogic_thermal_millicelsius_to_code(struct amlogic_thermal *pdata, int millicelsius)
> +{
> + const struct amlogic_thermal_soc_calib_data *param =
> + pdata->data->calibration_parameters;
> + s64 factor, uptat, uefuse;
> + u32 temp_code;
> +
> + uefuse = pdata->trim_info & TSENSOR_TRIM_SIGN_MASK ?
> + ~(pdata->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 :
> + (pdata->trim_info & TSENSOR_TRIM_TEMP_MASK);
> +
> + factor = param->B + div_s64(millicelsius, 100);
> + factor = BIT(16) * factor;
> + factor = div_s64(factor, param->A);
> + factor = factor - (uefuse & TSENSOR_TRIM_TEMP_MASK);
[Severity: High]
Does this bitwise mask strip the negative sign extension?
The uefuse variable can hold a negative calibration trim, but applying
TSENSOR_TRIM_TEMP_MASK here seems to destroy the negative offset, which
could cause extreme positive values and corrupt the reset temperature logic.
> +
> + uptat = param->n * factor;
> + uptat = div_s64(uptat, BIT(16));
> + uptat = param->m - uptat;
> +
> + factor = factor * 100;
> + factor = div_s64(factor, uptat);
[Severity: High]
Is there a risk of a divide-by-zero kernel panic here?
If the 'amlogic,hw-reset-temp' device tree property is set to a value around
981,000 mC, it looks like uptat can evaluate to zero. Should we validate the
reset temperature bounds before using it as a divisor?
> +
> + temp_code = ((factor >> 0x4) & TSENSOR_TEMP_MASK) + TSENSOR_TEMP_CAL;
[Severity: Medium]
Could this bitwise mask truncate valid temperature codes?
If temp_code evaluates to 13 bits, it would exceed the 12-bit bounds of
TSENSOR_TEMP_MASK. This might lead to a zeroed reset configuration at high
temperatures.
> +
> + return temp_code;
> +}
[ ... ]
> @@ -323,9 +398,24 @@ static int amlogic_thermal_probe(struct platform_device *pdev)
[ ... ]
> pdata->tzd = devm_thermal_of_zone_register(&pdev->dev,
> 0,
> pdata,
> &amlogic_thermal_ops);
[Severity: High]
This isn't a bug introduced by this patch, but does registering the thermal
zone here expose it prematurely?
Using devm_thermal_of_zone_register() before clocks are fully enabled allows
the core to poll the device immediately, which could cause a bus hang on
unclocked hardware access. Cleanup will also happen after clocks are disabled.
> if (IS_ERR(pdata->tzd)) {
> ret = PTR_ERR(pdata->tzd);
> dev_err(dev, "Failed to register tsensor: %d\n", ret);
> return ret;
> }
>
> devm_thermal_add_hwmon_sysfs(&pdev->dev, pdata->tzd);
>
> + pdata->temp_code = amlogic_thermal_millicelsius_to_code(pdata, reset_temp);
> +
> + amlogic_tsensor_setup_hw_reset(pdata);
[Severity: High]
Does this unconditionally apply A9-specific hardware over-temperature
configuration to all legacy SoCs?
Since this function writes to TSENSOR_CFG_REG2, which appears to be
A9-specific, applying it across all platforms without checking the SoC type
might cause undefined behavior on legacy hardware.
> +
> ret = amlogic_thermal_enable(pdata);
[ ... ]
> @@ -355,6 +449,8 @@ static void amlogic_thermal_remove(struct platform_device *pdev)
> {
> struct amlogic_thermal *data = platform_get_drvdata(pdev);
>
> + if (data->data->has_sysclk)
> + clk_disable_unprepare(data->sysclk);
[Severity: High]
Can this cause a reference count underflow for sysclk?
The sysclk was acquired in amlogic_thermal_probe() using
devm_clk_get_enabled(). Manually disabling a devm-managed clock here in the
remove callback could cause a double disable when the devm cleanup runs.
> amlogic_thermal_disable(data);
> }
[ ... ]
> @@ -371,6 +469,9 @@ static int amlogic_thermal_resume(struct device *dev)
> {
> struct amlogic_thermal *data = dev_get_drvdata(dev);
>
> + amlogic_tsensor_setup_hw_reset(data);
[Severity: High]
Will this cause a bus fault on unclocked MMIO access?
amlogic_tsensor_setup_hw_reset() accesses peripheral hardware registers via
regmap, but the backing clocks haven't been enabled yet. The clock enabling
seems to happen on the next line.
> + if (data->data->has_sysclk)
> + clk_prepare_enable(data->sysclk);
[Severity: High]
Should the return value of clk_prepare_enable() be checked here?
Ignoring the return value could lead to unbalanced clock references or
hardware access failures if enabling the sysclk fails during resume.
> return amlogic_thermal_enable(data);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-a9-thermal-v1-0-d786d37cba58@amlogic.com?part=2
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/2] thermal: amlogic: Add support for A9 thermal controller
@ 2026-07-30 8:46 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-07-30 8:46 UTC (permalink / raw)
To: Xianwei Zhao; +Cc: linux-amlogic, neil.armstrong, robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
New issues:
- [High] Double disable of 'sysclk' due to manual disable of devm-managed clock in remove callback.
- [High] Peripheral hardware registers are accessed via MMIO before their backing clocks are enabled.
- [High] Incorrect hardware reset temperature calculation for negative calibration trims due to flawed bitwise masking.
- [High] A9-specific hardware over-temperature reset configuration is applied unconditionally to all legacy SoCs.
- [High] The return value of `clk_prepare_enable(data->sysclk)` is ignored in the resume path.
- [High] An unvalidated device tree property can cause a divide-by-zero kernel panic during probe.
- [Medium] Possible truncation of `temp_code` during hardware reset configuration due to out-of-bounds bitwise masking.
Pre-existing issues:
- [High] The thermal zone is registered and unregistered in an unsafe order relative to hardware clock state.
--
--- Patch [2]: [PATCH 2/2] thermal: amlogic: Add support for A9 thermal controller ---
commit 0d417d06a989692f70ed92067eec1c72408bd3da
Author: Xianwei Zhao <xianwei.zhao@amlogic.com>
thermal: amlogic: Add support for A9 thermal controller
Add support for the Amlogic A9 thermal controller, utilizing different
calibration parameters, separate core and bus clocks, and a hardware
over-temperature reset mechanism. The hardware reset configuration is
also restored after resume.
> diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
> index a0b530624b60c..c9c3514966533 100644
> --- a/drivers/thermal/amlogic_thermal.c
> +++ b/drivers/thermal/amlogic_thermal.c
[ ... ]
> @@ -138,6 +154,46 @@ static int amlogic_thermal_code_to_millicelsius(struct amlogic_thermal *pdata,
> return temp;
> }
>
> +/*
> + * Calculate a temperature code from a temperature value .
> + * The unit of the temperature is degree milliCelsius.
> + */
> +static u32 amlogic_thermal_millicelsius_to_code(struct amlogic_thermal *pdata, int millicelsius)
> +{
> + const struct amlogic_thermal_soc_calib_data *param =
> + pdata->data->calibration_parameters;
> + s64 factor, uptat, uefuse;
> + u32 temp_code;
> +
> + uefuse = pdata->trim_info & TSENSOR_TRIM_SIGN_MASK ?
> + ~(pdata->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 :
> + (pdata->trim_info & TSENSOR_TRIM_TEMP_MASK);
> +
> + factor = param->B + div_s64(millicelsius, 100);
> + factor = BIT(16) * factor;
> + factor = div_s64(factor, param->A);
> + factor = factor - (uefuse & TSENSOR_TRIM_TEMP_MASK);
[Severity: High]
Does this bitwise mask strip the negative sign extension?
The uefuse variable can hold a negative calibration trim, but applying
TSENSOR_TRIM_TEMP_MASK here seems to destroy the negative offset, which
could cause extreme positive values and corrupt the reset temperature logic.
> +
> + uptat = param->n * factor;
> + uptat = div_s64(uptat, BIT(16));
> + uptat = param->m - uptat;
> +
> + factor = factor * 100;
> + factor = div_s64(factor, uptat);
[Severity: High]
Is there a risk of a divide-by-zero kernel panic here?
If the 'amlogic,hw-reset-temp' device tree property is set to a value around
981,000 mC, it looks like uptat can evaluate to zero. Should we validate the
reset temperature bounds before using it as a divisor?
> +
> + temp_code = ((factor >> 0x4) & TSENSOR_TEMP_MASK) + TSENSOR_TEMP_CAL;
[Severity: Medium]
Could this bitwise mask truncate valid temperature codes?
If temp_code evaluates to 13 bits, it would exceed the 12-bit bounds of
TSENSOR_TEMP_MASK. This might lead to a zeroed reset configuration at high
temperatures.
> +
> + return temp_code;
> +}
[ ... ]
> @@ -323,9 +398,24 @@ static int amlogic_thermal_probe(struct platform_device *pdev)
[ ... ]
> pdata->tzd = devm_thermal_of_zone_register(&pdev->dev,
> 0,
> pdata,
> &amlogic_thermal_ops);
[Severity: High]
This isn't a bug introduced by this patch, but does registering the thermal
zone here expose it prematurely?
Using devm_thermal_of_zone_register() before clocks are fully enabled allows
the core to poll the device immediately, which could cause a bus hang on
unclocked hardware access. Cleanup will also happen after clocks are disabled.
> if (IS_ERR(pdata->tzd)) {
> ret = PTR_ERR(pdata->tzd);
> dev_err(dev, "Failed to register tsensor: %d\n", ret);
> return ret;
> }
>
> devm_thermal_add_hwmon_sysfs(&pdev->dev, pdata->tzd);
>
> + pdata->temp_code = amlogic_thermal_millicelsius_to_code(pdata, reset_temp);
> +
> + amlogic_tsensor_setup_hw_reset(pdata);
[Severity: High]
Does this unconditionally apply A9-specific hardware over-temperature
configuration to all legacy SoCs?
Since this function writes to TSENSOR_CFG_REG2, which appears to be
A9-specific, applying it across all platforms without checking the SoC type
might cause undefined behavior on legacy hardware.
> +
> ret = amlogic_thermal_enable(pdata);
[ ... ]
> @@ -355,6 +449,8 @@ static void amlogic_thermal_remove(struct platform_device *pdev)
> {
> struct amlogic_thermal *data = platform_get_drvdata(pdev);
>
> + if (data->data->has_sysclk)
> + clk_disable_unprepare(data->sysclk);
[Severity: High]
Can this cause a reference count underflow for sysclk?
The sysclk was acquired in amlogic_thermal_probe() using
devm_clk_get_enabled(). Manually disabling a devm-managed clock here in the
remove callback could cause a double disable when the devm cleanup runs.
> amlogic_thermal_disable(data);
> }
[ ... ]
> @@ -371,6 +469,9 @@ static int amlogic_thermal_resume(struct device *dev)
> {
> struct amlogic_thermal *data = dev_get_drvdata(dev);
>
> + amlogic_tsensor_setup_hw_reset(data);
[Severity: High]
Will this cause a bus fault on unclocked MMIO access?
amlogic_tsensor_setup_hw_reset() accesses peripheral hardware registers via
regmap, but the backing clocks haven't been enabled yet. The clock enabling
seems to happen on the next line.
> + if (data->data->has_sysclk)
> + clk_prepare_enable(data->sysclk);
[Severity: High]
Should the return value of clk_prepare_enable() be checked here?
Ignoring the return value could lead to unbalanced clock references or
hardware access failures if enabling the sysclk fails during resume.
> return amlogic_thermal_enable(data);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-a9-thermal-v1-0-d786d37cba58@amlogic.com?part=2
^ permalink raw reply [flat|nested] 15+ messages in thread