[PATCH 0/3] thermal/drivers/amlogic: add S4 support

[PATCH 0/3] thermal/drivers/amlogic: add S4 support

[christian.koever-draxl]

From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

The S4 CPU sensor is the G12A design with the calibration read through
the secure monitor, as on T7, so it reuses the T7 data on top of Ronald
Claveau's secure monitor calibration support. Without calibration the
sensor reported about -144 degrees Celsius.

Patch 3 makes the expected first probe deferral (the secure monitor
probes later) quiet.

Tool use (Documentation/process/generated-content.rst):
This work was done with the help of an AI coding assistant, Claude
(Anthropic), used through Claude Code over many sessions. It did a large
part of the hands-on work: it read the vendor kernel and the firmware
interfaces, probed registers on the board, drafted the code and the
changelogs, and ran the builds, checkpatch, dt_binding_check and the
tests on the hardware. I directed the work, reviewed every change and
the final patches, and take responsibility for them. There is no single
prompt to quote; the sessions were of the form "find out why X fails on
this board, fix it, test it on the box".

Tested on an Amediatech X98Q TV box (Amlogic S905W2, S4 family, 2 GiB),
vendor U-Boot and BL31/OP-TEE, Linux 7.3 plus this work.

Christian Stefan Kövér-Draxl (3):
  dt-bindings: thermal: amlogic: add the S4 CPU sensor
  thermal/drivers/amlogic: add S4 support
  thermal/drivers/amlogic: do not log the secure monitor deferral as an
    error

 .../devicetree/bindings/thermal/amlogic,thermal.yaml  |  1 +
 drivers/thermal/amlogic_thermal.c                     | 11 +++++++----
 2 files changed, 8 insertions(+), 4 deletions(-)


base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
-- 
2.56.0


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

[PATCH 1/3] dt-bindings: thermal: amlogic: add the S4 CPU sensor

[christian.koever-draxl]

From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

The S4 CPU temperature sensor is the G12A design, but like on T7 its
calibration value is only available through the secure monitor: the
ao-secure register used on G12A holds unrelated data on S4. Add a
compatible for it; it takes a single clock and the secure monitor
phandle with the sensor index.

Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
---
 Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
index e72cb39ac635..5d966ab4d820 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,s4-cpu-thermal
           - amlogic,t7-thermal
 
   reg:
-- 
2.56.0


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

Re: [PATCH 1/3] dt-bindings: thermal: amlogic: add the S4 CPU sensor

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: thermal: amlogic: missing bus clock constraint for S4 sensor
--

commit b2d42a2f90c7882650efb090daff31a9ed1ad3e0
Author: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

dt-bindings: thermal: amlogic: add the S4 CPU sensor

This commit adds the amlogic,s4-cpu-thermal compatible string to the Amlogic
thermal binding schema for the S4 CPU temperature sensor.

> diff --git a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
> index e72cb39ac635d..5d966ab4d820e 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,s4-cpu-thermal
>            - amlogic,t7-thermal
>  
>    reg:

[Severity: Medium]
Should amlogic,s4-cpu-thermal also require two clocks (bus and core) in the
allOf conditional block, or use amlogic,t7-thermal as a fallback compatible?

In Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml, the second
allOf conditional checks only for amlogic,t7-thermal:

  - if:
      properties:
        compatible:
          contains:
            const: amlogic,t7-thermal
    then:
      properties:
        clocks:
          minItems: 2
      required:
        - clock-names
    else:
      properties:
        clocks:
          maxItems: 1
        clock-names: false

Because amlogic,s4-cpu-thermal is omitted from that condition, it falls into
the else branch, which restricts clocks to a single entry and forbids
clock-names.

In commit 8195b945794c ("dt-bindings: thermal: amlogic: describe bus clock
for t7-thermal"), the single-clock else branch was retained only for older
bindings to avoid breaking existing device trees, whereas T7 requires both
the peripheral bus gate clock and the shared core clock.

Looking at drivers/clk/meson/s4-peripherals.c, S4 has the same clock topology
as T7, with dedicated per-sensor bus gate clocks (s4_ts_ddr / CLKID_TS_DDR
and s4_ts_pll / CLKID_TS_PLL) in addition to the shared functional clock
(s4_ts_clk / CLKID_TS):

drivers/clk/meson/s4-peripherals.c:
static S4_PCLK(s4_ts_ddr,	CLKCTRL_SYS_CLK_EN0_REG1, 15, CLK_IGNORE_UNUSED);
static S4_PCLK(s4_ts_pll,	CLKCTRL_SYS_CLK_EN0_REG1, 16, CLK_IGNORE_UNUSED);

Without allowing both clocks in the binding, device tree nodes using
amlogic,s4-cpu-thermal cannot describe the sensor's peripheral bus gate clock
for amlogic_thermal_probe() to manage via devm_clk_bulk_get_all(), leaving
the sensor reliant on the temporary CLK_IGNORE_UNUSED flag on s4_ts_pll.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025192244.209256-1-christian.koever-draxl@student.uibk.ac.at?part=1

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

[PATCH 2/3] thermal/drivers/amlogic: add S4 support

[christian.koever-draxl]

From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

The S4 CPU sensor reads its calibration through the secure monitor and
otherwise matches the T7 setup, so reuse the T7 data for it.

Without the calibration the driver reported about -144 degrees Celsius
on an S905W2 board; with it the readings are plausible and CPU
throttling engages.

Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
---
 drivers/thermal/amlogic_thermal.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
index 60728de6968a..c58bac633bed 100644
--- a/drivers/thermal/amlogic_thermal.c
+++ b/drivers/thermal/amlogic_thermal.c
@@ -292,6 +292,10 @@ static const struct of_device_id of_amlogic_thermal_match[] = {
 		.compatible = "amlogic,a1-cpu-thermal",
 		.data = &amlogic_thermal_a1_cpu_param,
 	},
+	{
+		.compatible = "amlogic,s4-cpu-thermal",
+		.data = &amlogic_thermal_t7_param,
+	},
 	{
 		.compatible = "amlogic,t7-thermal",
 		.data = &amlogic_thermal_t7_param,
-- 
2.56.0


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

Re: [PATCH 2/3] thermal/drivers/amlogic: add S4 support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] thermal/drivers/amlogic: missing S4 bus clock in binding and compatible
--

commit dfe5abf7967aecb95213a29d43ea6efa65ceb5d9
Author: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

thermal/drivers/amlogic: add S4 support

This commit adds the amlogic,s4-cpu-thermal compatible string to the Amlogic
thermal driver match table, reusing the T7 parameter data so that the S4 CPU
thermal sensor reads its calibration via the secure monitor.

> diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
> index 60728de6968ac..c58bac633bedc 100644
> --- a/drivers/thermal/amlogic_thermal.c
> +++ b/drivers/thermal/amlogic_thermal.c
> @@ -292,6 +292,10 @@ static const struct of_device_id of_amlogic_thermal_match[] = {
>  		.compatible = "amlogic,a1-cpu-thermal",
>  		.data = &amlogic_thermal_a1_cpu_param,
>  	},
> +	{
> +		.compatible = "amlogic,s4-cpu-thermal",
> +		.data = &amlogic_thermal_t7_param,
> +	},

[Severity: Medium]
Should S4 use amlogic,s4-thermal (either with amlogic,t7-thermal as a
fallback compatible or added to the two-clock condition in the binding)
instead of a standalone amlogic,s4-cpu-thermal compatible?

In drivers/clk/meson/s4-peripherals.c, Amlogic S4 provides both a shared
functional clock (s4_ts_clk) and dedicated per-sensor APB bus clocks for the
PLL/CPU and DDR thermal sensors:

static S4_PCLK(s4_ts_ddr,	CLKCTRL_SYS_CLK_EN0_REG1, 15, CLK_IGNORE_UNUSED);
static S4_PCLK(s4_ts_pll,	CLKCTRL_SYS_CLK_EN0_REG1, 16, CLK_IGNORE_UNUSED);

However, in Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml,
the allOf conditional block only permits two clocks when the compatible
contains amlogic,t7-thermal:

  - if:
      properties:
        compatible:
          contains:
            const: amlogic,t7-thermal
    then:
      properties:
        clocks:
          minItems: 2
      required:
        - clock-names
    else:
      properties:
        clocks:
          maxItems: 1
        clock-names: false

Because amlogic,s4-cpu-thermal falls into the else branch, the schema forbids
specifying the per-sensor APB bus clock (CLKID_TS_PLL / CLKID_TS_DDR) alongside
CLKID_TS. If the bootloader does not leave the APB bus clock enabled, or once
CLK_IGNORE_UNUSED is removed from s4_ts_pll and s4_ts_ddr, register accesses in
amlogic_thermal_enable() and amlogic_thermal_get_temp() during
amlogic_thermal_probe() can access an unclocked peripheral.

Additionally, since S4 passes the sensor index via the amlogic,secure-monitor
phandle argument rather than hardcoding u_efuse_off per compatible, does
naming the compatible amlogic,s4-cpu-thermal prevent reusing it for the S4 DDR
thermal sensor?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025192244.209256-1-christian.koever-draxl@student.uibk.ac.at?part=2

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

[PATCH 3/3] thermal/drivers/amlogic: do not log the secure monitor deferral as an error

[christian.koever-draxl]

From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

The thermal sensor usually probes before the secure monitor, so the
first attempt always defers and logs "Failed to get secure monitor
firmware" as an error on every boot, although the second attempt
succeeds. Use dev_err_probe(), which records the reason for the
deferral instead of logging it.

Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
---
 drivers/thermal/amlogic_thermal.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
index c58bac633bed..54e59f444d46 100644
--- a/drivers/thermal/amlogic_thermal.c
+++ b/drivers/thermal/amlogic_thermal.c
@@ -196,10 +196,9 @@ static int amlogic_thermal_probe_sm(struct platform_device *pdev,
 
 	pdata->sm_fw = meson_sm_get(ph_args.np);
 	of_node_put(ph_args.np);
-	if (!pdata->sm_fw) {
-		dev_err(dev, "Failed to get secure monitor firmware\n");
-		return -EPROBE_DEFER;
-	}
+	if (!pdata->sm_fw)
+		return dev_err_probe(dev, -EPROBE_DEFER,
+				     "Failed to get secure monitor firmware\n");
 
 	pdata->tsensor_id = ph_args.args[0];
 
-- 
2.56.0


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

Re: [PATCH 3/3] thermal/drivers/amlogic: do not log the secure monitor deferral as an error

[neil.armstrong]

On 10/25/26 20:22, christian.koever-draxl@student.uibk.ac.at wrote:
> From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
> 
> The thermal sensor usually probes before the secure monitor, so the
> first attempt always defers and logs "Failed to get secure monitor
> firmware" as an error on every boot, although the second attempt
> succeeds. Use dev_err_probe(), which records the reason for the
> deferral instead of logging it.
> 
> Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
> ---
>   drivers/thermal/amlogic_thermal.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
> index c58bac633bed..54e59f444d46 100644
> --- a/drivers/thermal/amlogic_thermal.c
> +++ b/drivers/thermal/amlogic_thermal.c
> @@ -196,10 +196,9 @@ static int amlogic_thermal_probe_sm(struct platform_device *pdev,
>   
>   	pdata->sm_fw = meson_sm_get(ph_args.np);
>   	of_node_put(ph_args.np);
> -	if (!pdata->sm_fw) {
> -		dev_err(dev, "Failed to get secure monitor firmware\n");
> -		return -EPROBE_DEFER;
> -	}
> +	if (!pdata->sm_fw)
> +		return dev_err_probe(dev, -EPROBE_DEFER,
> +				     "Failed to get secure monitor firmware\n");
>   
>   	pdata->tsensor_id = ph_args.args[0];
>   

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

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