[PATCH v5 0/2] thermal: Add support A9

[PATCH v5 0/2] thermal: Add support A9

[Xianwei Zhao via B4 Relay]

Document the thermal controller compatibles for the Amlogic A9 SoC
And add driver support for the Amlogic A9 thermal controller.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
Changes in v5:
- Check enable clock func returns and set the restart time of register separately.
- Link to v4: https://lore.kernel.org/r/20260825-a9-thermal-v4-0-d8fc9859cf22@amlogic.com

Changes in v4:
- Fix type error in comit mesage, and set factor valid value. 
- Link to v3: https://lore.kernel.org/r/20260821-a9-thermal-v3-0-0e7abf3941e3@amlogic.com

Changes in v3:
- Binding: add restrictive conditions for clock, and remove remove "amlogic,hw-reset-temp" property.
- Driver: add private data reset_temp instead of get value from DTS.
- Link to v2: https://lore.kernel.org/r/20260811-a9-thermal-v2-0-7ddc33b11b9a@amlogic.com

Changes in v2:
- Fix the spelling mistakes (clock-names) and reset-temp properties is visible to old chips.
- Adjust the clock enable sequence in resume func and remove the clock disable in remove func.
- Add the judgment on the divisor zero.
- Add description of support for over-temperature reset for old chips in commit message.
- Link to v1: https://lore.kernel.org/r/20260730-a9-thermal-v1-0-d786d37cba58@amlogic.com

---
Xianwei Zhao (2):
      dt-bindings: thermal: amlogic: Add A9 thermal bindings
      thermal: amlogic: Add support for A9 thermal controller

 .../bindings/thermal/amlogic,thermal.yaml          |  27 ++++-
 drivers/thermal/amlogic_thermal.c                  | 121 ++++++++++++++++++++-
 2 files changed, 144 insertions(+), 4 deletions(-)
---
base-commit: 78bc8af4affb9a732504eb22eeac7d1e50883853
change-id: 20260703-a9-thermal-f62cef6ea91f

Best regards,
-- 
Xianwei Zhao <xianwei.zhao@amlogic.com>



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v5 1/2] dt-bindings: thermal: amlogic: Add A9 thermal bindings

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Document the thermal controller compatibles for the Amlogic A9 SoC.

Describe the required "core" and "pclk" clocks, and update the schema
constraints for A9-specific thermal controllers.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 .../bindings/thermal/amlogic,thermal.yaml          | 27 +++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
index 8cfa44dcda58..1b02404d67d5 100644
--- a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
@@ -23,6 +23,7 @@ properties:
           - const: amlogic,g12a-thermal
       - enum:
           - amlogic,a1-cpu-thermal
+          - amlogic,a9-thermal
           - amlogic,t7-thermal
 
   reg:
@@ -32,7 +33,13 @@ properties:
     maxItems: 1
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: core
+      - const: pclk
 
   power-domains:
     maxItems: 1
@@ -73,6 +80,24 @@ allOf:
       required:
         - amlogic,secure-monitor
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amlogic,a9-thermal
+    then:
+      properties:
+        clocks:
+          minItems: 2
+      required:
+        - clock-names
+    else:
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names: false
+
 unevaluatedProperties: false
 
 examples:

-- 
2.52.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v5 2/2] thermal: amlogic: Add support for A9 thermal controller

[Xianwei Zhao via B4 Relay]

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, and restore the hardware reset configuration after resume.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 drivers/thermal/amlogic_thermal.c | 121 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 118 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
index a0b530624b60..e7cedb944315 100644
--- a/drivers/thermal/amlogic_thermal.c
+++ b/drivers/thermal/amlogic_thermal.c
@@ -45,6 +45,19 @@
 		 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)
+
+#define TSENSOR_REBOOT_TIME_MAX		(0xff << 16)
+
+#define TSENSOR_TEMP_CAL		1
+
 #define TSENSOR_STAT0			0x40
 
 #define TSENSOR_STAT9			0x64
@@ -63,6 +76,7 @@
 
 #define TSENSOR_CALIB_OFFSET	1
 #define TSENSOR_CALIB_SHIFT	4
+#define TSENSOR_HW_RESET_DEFAULT_TEMP	110000
 
 /**
  * struct amlogic_thermal_soc_calib_data
@@ -85,14 +99,18 @@ struct amlogic_thermal_soc_calib_data {
  * @u_efuse_off: register offset to read fused calibration value
  * @calibration_parameters: calibration parameters structure pointer
  * @regmap_config: regmap config for the device
+ * @reset_temp: the hardware-controlled reset temperature value
  * @use_sm: read data from secure monitor instead of efuse
+ * @has_sysclk: sensor driver control sysclk
  * This structure is required for configuration of amlogic thermal driver.
  */
 struct amlogic_thermal_data {
 	int u_efuse_off;
 	const struct amlogic_thermal_soc_calib_data *calibration_parameters;
 	const struct regmap_config *regmap_config;
+	u32 reset_temp;
 	bool use_sm;
+	bool has_sysclk;
 };
 
 struct amlogic_thermal {
@@ -101,8 +119,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 +158,51 @@ 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;
+
+	uptat = param->n * factor;
+	uptat = div_s64(uptat, BIT(16));
+	uptat = param->m - uptat;
+
+	factor = factor * 100;
+	if (!uptat)
+		factor = (TSENSOR_TEMP_MASK - TSENSOR_TEMP_CAL) << 0x4;
+	else
+		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_REBOOT_TIME,
+			   TSENSOR_REBOOT_TIME_MAX);
+	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 +319,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 +344,14 @@ 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,
+	.reset_temp		= 110000,
+};
+
 static const struct amlogic_thermal_data amlogic_thermal_t7_param = {
 	.use_sm			= true,
 	.calibration_parameters	= &amlogic_thermal_g12a,
@@ -291,6 +371,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 +389,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 +408,18 @@ 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 (pdata->data->use_sm)
 		ret = amlogic_thermal_probe_sm(pdev, pdata);
@@ -346,6 +440,17 @@ static int amlogic_thermal_probe(struct platform_device *pdev)
 
 	devm_thermal_add_hwmon_sysfs(&pdev->dev, pdata->tzd);
 
+	if (pdata->data->reset_temp) {
+		reset_temp = pdata->data->reset_temp;
+	} else {
+		dev_dbg(dev, "using default hardware reset temperature\n");
+		reset_temp = TSENSOR_HW_RESET_DEFAULT_TEMP;
+	}
+
+	pdata->temp_code = amlogic_thermal_millicelsius_to_code(pdata, reset_temp);
+
+	amlogic_tsensor_setup_hw_reset(pdata);
+
 	ret = amlogic_thermal_enable(pdata);
 
 	return ret;
@@ -363,6 +468,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;
 }
@@ -370,7 +477,15 @@ static int amlogic_thermal_suspend(struct device *dev)
 static int amlogic_thermal_resume(struct device *dev)
 {
 	struct amlogic_thermal *data = dev_get_drvdata(dev);
+	int ret;
+
+	if (data->data->has_sysclk) {
+		ret = clk_prepare_enable(data->sysclk);
+		if (ret)
+			return ret;
+	}
 
+	amlogic_tsensor_setup_hw_reset(data);
 	return amlogic_thermal_enable(data);
 }
 

-- 
2.52.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v5 2/2] thermal: amlogic: Add support for A9 thermal controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Critical] The A9-specific hardware over-temperature reset mechanism is enabled unconditionally on all SoCs, causing unclocked MMIO accesses and undefined register writes that will crash existing platforms (e.g., G12A) on resume.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the thermal zone and hwmon sysfs interfaces are registered before the hardware and clocks are fully initialized, creating a race condition that leads to unclocked MMIO reads.
- [High] This problem wasn't introduced by this patch, but incorrect sign extension of a bitwise NOT operation on a 32-bit unsigned mask causes massively inflated negative trim values on 32-bit architectures, breaking thermal calibration.
- [High] This problem wasn't introduced by this patch, but disabling hardware clocks in `remove()` before the devres-managed thermal zone is unregistered introduces a use-after-disable race condition that can crash the system.
--

commit c409169ce8a3938d784d6682dd656715a5d6d606
Author: Xianwei Zhao <xianwei.zhao@amlogic.com>

thermal: amlogic: Add support for A9 thermal controller

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. It also restores the hardware reset configuration after
resume.

> diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
> index a0b530624b60c..e7cedb9443151 100644
> --- a/drivers/thermal/amlogic_thermal.c
> +++ b/drivers/thermal/amlogic_thermal.c
[ ... ]
> @@ -138,6 +158,51 @@ 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);

[Severity: High]
This is a pre-existing issue, but by copying this logic from
amlogic_thermal_code_to_millicelsius(), the new function inherits a sign
extension bug.

When the hardware provides a negative trim value (bit 15 set in trim_info),
the bitwise NOT operation on the 32-bit unsigned TSENSOR_TRIM_TEMP_MASK mask
evaluates to a large positive value on 32-bit systems (e.g., 0xFFFFFFFF).

When this is assigned to the s64 uefuse variable, C promotion rules
zero-extend it to 4294967295 rather than sign-extending to -1.

This breaks thermal calibration for hardware instances with negative factory
calibration offsets, rendering those devices unusable.

Could we fix this integer conversion so that the negative value is properly
sign-extended into the s64 variable?

[ ... ]
> @@ -346,6 +440,17 @@ static int amlogic_thermal_probe(struct platform_device *pdev)
>  
>  	devm_thermal_add_hwmon_sysfs(&pdev->dev, pdata->tzd);
>  

[Severity: High]
This isn't a bug introduced by this patch, but registering the thermal zone
device here before the hardware clocks are fully initialized creates a race
condition.

The thermal core invokes the .get_temp callback immediately upon registration
to establish the initial temperature state.

If the driver is built as a loadable module and loaded after boot, when
bootloader-enabled clocks have already been disabled by the clock framework's
late init, the .get_temp call will attempt to read from the hardware before
its clock is prepared and enabled.

Will this result in an unclocked MMIO read and subsequent bus fault?

> +	if (pdata->data->reset_temp) {
> +		reset_temp = pdata->data->reset_temp;
> +	} else {
> +		dev_dbg(dev, "using default hardware reset temperature\n");
> +		reset_temp = TSENSOR_HW_RESET_DEFAULT_TEMP;
> +	}
> +
> +	pdata->temp_code = amlogic_thermal_millicelsius_to_code(pdata, reset_temp);
> +
> +	amlogic_tsensor_setup_hw_reset(pdata);
> +
>  	ret = amlogic_thermal_enable(pdata);

[Severity: Critical]
Does unconditionally applying the A9-specific hardware reset setup here
cause issues for older non-A9 platforms?

For existing SoCs where has_sysclk is false, the peripheral clock (data->clk)
isn't enabled until amlogic_thermal_enable() is called on the next line.

Since the clock is physically off during probe, does this regmap_update_bits()
write inside amlogic_tsensor_setup_hw_reset() trigger a bus fault?

Also, could writing to TSENSOR_CFG_REG2 cause undefined register modification
on hardware that doesn't support this over-temperature reset mechanism?

[ ... ]
> @@ -370,7 +477,15 @@ static int amlogic_thermal_suspend(struct device *dev)
>  static int amlogic_thermal_resume(struct device *dev)
>  {
>  	struct amlogic_thermal *data = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (data->data->has_sysclk) {
> +		ret = clk_prepare_enable(data->sysclk);
> +		if (ret)
> +			return ret;
> +	}
>  
> +	amlogic_tsensor_setup_hw_reset(data);
>  	return amlogic_thermal_enable(data);

[Severity: Critical]
Similarly, does this execute an MMIO write before the peripheral clock is
enabled during the resume path for non-A9 platforms?

On non-A9 platforms, data->clk is only enabled inside
amlogic_thermal_enable().

If this write occurs while the clock is still physically disabled during
system resume, could this cause a synchronous external abort and break power
management completely?

[Severity: High]
This is a pre-existing issue, but is there a use-after-disable race condition
in the driver's remove path?

drivers/thermal/amlogic_thermal.c:amlogic_thermal_remove() {
    struct amlogic_thermal *data = platform_get_drvdata(pdev);

    amlogic_thermal_disable(data);
}

Disabling the hardware clocks in remove() before the devres-managed thermal
zone is cleanly unregistered could crash the system.

Because pdata->tzd was registered with devm_thermal_of_zone_register(), it
remains fully active and exposed to the thermal core until the devres cleanup
phase, which executes after remove() returns.

If the thermal core's polling timer fires, or if a user reads the temperature
sysfs attribute during this window, amlogic_thermal_get_temp() will attempt
to read from the unclocked hardware.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-a9-thermal-v5-0-892d798723e1@amlogic.com?part=2

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v5 2/2] thermal: amlogic: Add support for A9 thermal controller

[Xianwei Zhao]

On 2026/9/10 15:06, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> 
> New issues:
> - [Critical] The A9-specific hardware over-temperature reset mechanism is enabled unconditionally on all SoCs, causing unclocked MMIO accesses and undefined register writes that will crash existing platforms (e.g., G12A) on resume.
> 
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but the thermal zone and hwmon sysfs interfaces are registered before the hardware and clocks are fully initialized, creating a race condition that leads to unclocked MMIO reads.
> - [High] This problem wasn't introduced by this patch, but incorrect sign extension of a bitwise NOT operation on a 32-bit unsigned mask causes massively inflated negative trim values on 32-bit architectures, breaking thermal calibration.
> - [High] This problem wasn't introduced by this patch, but disabling hardware clocks in `remove()` before the devres-managed thermal zone is unregistered introduces a use-after-disable race condition that can crash the system.
> --
> 
> commit c409169ce8a3938d784d6682dd656715a5d6d606
> Author: Xianwei Zhao<xianwei.zhao@amlogic.com>
> 
> thermal: amlogic: Add support for A9 thermal controller
> 
> 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. It also restores the hardware reset configuration after
> resume.
> 
>> diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
>> index a0b530624b60c..e7cedb9443151 100644
>> --- a/drivers/thermal/amlogic_thermal.c
>> +++ b/drivers/thermal/amlogic_thermal.c
> [ ... ]
>> @@ -138,6 +158,51 @@ 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);
> [Severity: High]
> This is a pre-existing issue, but by copying this logic from
> amlogic_thermal_code_to_millicelsius(), the new function inherits a sign
> extension bug.
> 
> When the hardware provides a negative trim value (bit 15 set in trim_info),
> the bitwise NOT operation on the 32-bit unsigned TSENSOR_TRIM_TEMP_MASK mask
> evaluates to a large positive value on 32-bit systems (e.g., 0xFFFFFFFF).
> 
> When this is assigned to the s64 uefuse variable, C promotion rules
> zero-extend it to 4294967295 rather than sign-extending to -1.
> 
> This breaks thermal calibration for hardware instances with negative factory
> calibration offsets, rendering those devices unusable.
> 
> Could we fix this integer conversion so that the negative value is properly
> sign-extended into the s64 variable?
> 
> [ ... ]
>> @@ -346,6 +440,17 @@ static int amlogic_thermal_probe(struct platform_device *pdev)
>>
>>        devm_thermal_add_hwmon_sysfs(&pdev->dev, pdata->tzd);
>>
> [Severity: High]
> This isn't a bug introduced by this patch, but registering the thermal zone
> device here before the hardware clocks are fully initialized creates a race
> condition.
> 
> The thermal core invokes the .get_temp callback immediately upon registration
> to establish the initial temperature state.
> 
> If the driver is built as a loadable module and loaded after boot, when
> bootloader-enabled clocks have already been disabled by the clock framework's
> late init, the .get_temp call will attempt to read from the hardware before
> its clock is prepared and enabled.
> 
> Will this result in an unclocked MMIO read and subsequent bus fault?
> 
>> +     if (pdata->data->reset_temp) {
>> +             reset_temp = pdata->data->reset_temp;
>> +     } else {
>> +             dev_dbg(dev, "using default hardware reset temperature\n");
>> +             reset_temp = TSENSOR_HW_RESET_DEFAULT_TEMP;
>> +     }
>> +
>> +     pdata->temp_code = amlogic_thermal_millicelsius_to_code(pdata, reset_temp);
>> +
>> +     amlogic_tsensor_setup_hw_reset(pdata);
>> +
>>        ret = amlogic_thermal_enable(pdata);
> [Severity: Critical]
> Does unconditionally applying the A9-specific hardware reset setup here
> cause issues for older non-A9 platforms?
> 

Non-A9 HW is support too, just that this function wasn't added before. 
Here, the default value is used.

> For existing SoCs where has_sysclk is false, the peripheral clock (data->clk)
> isn't enabled until amlogic_thermal_enable() is called on the next line.
> 

The existing SoCs sys clock is not control by this driver, it is enabled 
by default.

> Since the clock is physically off during probe, does this regmap_update_bits()
> write inside amlogic_tsensor_setup_hw_reset() trigger a bus fault?
> 
> Also, could writing to TSENSOR_CFG_REG2 cause undefined register modification
> on hardware that doesn't support this over-temperature reset mechanism?
> 
The Non-SoC has TSENSOR_CFG_REG2 regiter, the register was hidden before 
because the related functions were not supported.

> [ ... ]
>> @@ -370,7 +477,15 @@ static int amlogic_thermal_suspend(struct device *dev)
>>   static int amlogic_thermal_resume(struct device *dev)
>>   {
>>        struct amlogic_thermal *data = dev_get_drvdata(dev);
>> +     int ret;
>> +
>> +     if (data->data->has_sysclk) {
>> +             ret = clk_prepare_enable(data->sysclk);
>> +             if (ret)
>> +                     return ret;
>> +     }
>>
>> +     amlogic_tsensor_setup_hw_reset(data);
>>        return amlogic_thermal_enable(data);
> [Severity: Critical]
> Similarly, does this execute an MMIO write before the peripheral clock is
> enabled during the resume path for non-A9 platforms?
> 
> On non-A9 platforms, data->clk is only enabled inside
> amlogic_thermal_enable().
> 

The sysclock is keeping openning all time for other SoC.


> If this write occurs while the clock is still physically disabled during
> system resume, could this cause a synchronous external abort and break power
> management completely?
> 

Non A9 clock is aways_on ,so for A9 if enable sysclock failed, it will 
be return.

> [Severity: High]
> This is a pre-existing issue, but is there a use-after-disable race condition
> in the driver's remove path?
> 
> drivers/thermal/amlogic_thermal.c:amlogic_thermal_remove() {
>      struct amlogic_thermal *data = platform_get_drvdata(pdev);
> 
>      amlogic_thermal_disable(data);
> }
> 
> Disabling the hardware clocks in remove() before the devres-managed thermal
> zone is cleanly unregistered could crash the system.
> 
> Because pdata->tzd was registered with devm_thermal_of_zone_register(), it
> remains fully active and exposed to the thermal core until the devres cleanup
> phase, which executes after remove() returns.
> 
> If the thermal core's polling timer fires, or if a user reads the temperature
> sysfs attribute during this window, amlogic_thermal_get_temp() will attempt
> to read from the unclocked hardware.

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic