[PATCH 0/2] Add bus clock for t7-thermal sensor

[PATCH 0/2] Add bus clock for t7-thermal sensor

[Ronald Claveau via B4 Relay]

The t7 thermal sensor requires a bus clock, gating register access to
the sensor, in addition to the functional clock that is already
described and shared between sensors on other Amlogic SoCs.

This series first extends the devicetree binding to describe this
second clock, making it mandatory only for the t7-thermal compatible
so other SoCs are unaffected, then updates the driver to use the
clk_bulk API instead of a single struct clk, so it can transparently
handle however many clocks a given compatible declares.

`dt_binding_check DT_SCHEMA_FILES=amlogic,thermal.yaml` is ok

  CHKDT   ./Documentation/devicetree/bindings
warning: python package 'yamllint' not installed, skipping
  STYLE   ./Documentation/devicetree/bindings
  DTEX    Documentation/devicetree/bindings/thermal/amlogic,thermal.example.dts
  DTC [C] Documentation/devicetree/bindings/thermal/amlogic,thermal.example.dtb

Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
Ronald Claveau (2):
      dt-bindings: thermal: amlogic: describe bus clock for t7-thermal
      thermal/drivers/amlogic: switch to clk_bulk API to support multiple clocks

 .../bindings/thermal/amlogic,thermal.yaml          | 29 ++++++++++++++++++++--
 drivers/thermal/amlogic_thermal.c                  | 14 ++++++-----
 2 files changed, 35 insertions(+), 8 deletions(-)
---
base-commit: a8c591ed6b672915e0be57843f943a2a723aff40
change-id: 20260922-fix-t7-thermal-clock-151a0bf4746e

Best regards,
-- 
Ronald Claveau <linux-kernel-dev@aliel.fr>

[PATCH 1/2] dt-bindings: thermal: amlogic: describe bus clock for t7-thermal

[Ronald Claveau via B4 Relay]

From: Ronald Claveau <linux-kernel-dev@aliel.fr>

The t7 thermal sensor needs a bus clock, in addition to the existing
functional clock, to gate register access to the sensor. This bus
clock is specific to each sensor, unlike the functional clock which
is shared between sensors. The current binding only describes a
single unnamed clock and therefore cannot represent this hardware
requirement, so add clock-names to disambiguate "bus" from "core" and
make the second clock mandatory for the t7-thermal compatible only,
via an additional if/then/else, to avoid breaking other Amlogic SoCs
that still use a single clock. Update the t7 example to match.

Fixes: b1c8ccdbd4e9 ("dt-bindings: thermal: amlogic: Add support for T7")
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
 .../bindings/thermal/amlogic,thermal.yaml          | 29 ++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
index 8cfa44dcda58c..e72cb39ac635d 100644
--- a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
@@ -32,7 +32,15 @@ properties:
     maxItems: 1
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: Bus clock, gates register access to the sensor.
+      - description: Functional clock of the temperature sensor, shared between sensors.
+
+  clock-names:
+    items:
+      - const: bus
+      - const: core
 
   power-domains:
     maxItems: 1
@@ -72,6 +80,22 @@ allOf:
     else:
       required:
         - amlogic,secure-monitor
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: amlogic,t7-thermal
+    then:
+      properties:
+        clocks:
+          minItems: 2
+      required:
+        - clock-names
+    else:
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names: false
 
 unevaluatedProperties: false
 
@@ -94,7 +118,8 @@ examples:
         compatible = "amlogic,t7-thermal";
         reg = <0x20000 0x50>;
         interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
-        clocks = <&clkc_periphs CLKID_TS>;
+        clocks = <&clkc_periphs CLKID_SYS_TS_A73>, <&clkc_periphs CLKID_TS>;
+        clock-names = "bus", "core";
         #thermal-sensor-cells = <0>;
         amlogic,secure-monitor = <&sm 1>;
     };

-- 
2.49.0

Re: [PATCH 1/2] dt-bindings: thermal: amlogic: describe bus clock for t7-thermal

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] dt-bindings: thermal: amlogic: ABI breakage by prepending new bus clock
--

commit 9cad8324bf204ceb98e53aadbfac54a0b8d04da6
Author: Ronald Claveau <linux-kernel-dev@aliel.fr>

dt-bindings: thermal: amlogic: describe bus clock for t7-thermal

This commit updates the amlogic thermal device tree binding to support a
secondary "bus" clock required by the t7-thermal sensor. It adds the
"clock-names" property to distinguish between the "bus" and "core" clocks
and requires both for the t7-thermal compatible.

> diff --git a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
> index 8cfa44dcda58c..e72cb39ac635d 100644
> --- a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
> @@ -32,7 +32,15 @@ properties:
>      maxItems: 1
>  
>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    items:
> +      - description: Bus clock, gates register access to the sensor.
> +      - description: Functional clock of the temperature sensor, shared between sensors.
> +
> +  clock-names:
> +    items:
> +      - const: bus
> +      - const: core

[Severity: Critical]
Does prepending the new "bus" clock at index 0 introduce an ABI regression?

Since older kernels use devm_clk_get(dev, NULL) to unconditionally fetch
the clock at index 0 (as seen in drivers/thermal/amlogic_thermal.c:
amlogic_thermal_probe()), updating the device tree to match this new schema
will cause those older kernels to fetch the "bus" clock instead of the
functional "core" clock.

This will leave the functional clock disabled and prevent the thermal sensor
from operating. While a later patch in this series updates the driver to use
devm_clk_bulk_get_all(), the device tree schema remains broken for older
kernels.

Would it be better to append the new "bus" clock at index 1 to avoid
breaking backward and forward compatibility?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-fix-t7-thermal-clock-v1-0-7d4c4682f182@aliel.fr?part=1

Re: [PATCH 1/2] dt-bindings: thermal: amlogic: describe bus clock for t7-thermal

[Krzysztof Kozlowski]

On 23/09/2026 13:51, Ronald Claveau via B4 Relay wrote:
> From: Ronald Claveau <linux-kernel-dev@aliel.fr>
> 
> The t7 thermal sensor needs a bus clock, in addition to the existing
> functional clock, to gate register access to the sensor. This bus
> clock is specific to each sensor, unlike the functional clock which
> is shared between sensors. The current binding only describes a
> single unnamed clock and therefore cannot represent this hardware
> requirement, so add clock-names to disambiguate "bus" from "core" and

Index distinguishes, not the clock-names because items have strict
order. That part of sentence is redundant and more important - if I ack
this patch, people will use it as an argument in the future review
discussions.

Drop this part of sentence. clock-names are fine if you need them, but
still the disambiguation is provided by indices.

> make the second clock mandatory for the t7-thermal compatible only


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: thermal: amlogic: describe bus clock for t7-thermal

[linux-kernel-dev]

On 9/23/26 2:02 PM, Krzysztof Kozlowski wrote:
> On 23/09/2026 13:51, Ronald Claveau via B4 Relay wrote:
>> From: Ronald Claveau <linux-kernel-dev@aliel.fr>
>>
>> The t7 thermal sensor needs a bus clock, in addition to the existing
>> functional clock, to gate register access to the sensor. This bus
>> clock is specific to each sensor, unlike the functional clock which
>> is shared between sensors. The current binding only describes a
>> single unnamed clock and therefore cannot represent this hardware
>> requirement, so add clock-names to disambiguate "bus" from "core" and
> 
> Index distinguishes, not the clock-names because items have strict
> order. That part of sentence is redundant and more important - if I ack
> this patch, people will use it as an argument in the future review
> discussions.
> 
> Drop this part of sentence. clock-names are fine if you need them, but
> still the disambiguation is provided by indices.
> 

Thanks for your feedback.
I will replace `so add clock-names to disambiguate "bus" from "core"` by
`so add a second clock entry and clock-names for clarity` in next version.

>> make the second clock mandatory for the t7-thermal compatible only
> 
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> 
> 
> Best regards,
> Krzysztof


-- 
Best regards,
Ronald

[PATCH 2/2] thermal/drivers/amlogic: switch to clk_bulk API to support multiple clocks

[Ronald Claveau via B4 Relay]

From: Ronald Claveau <linux-kernel-dev@aliel.fr>

The t7-thermal variant now exposes two clocks (bus and core) in its
devicetree binding, but the driver only knows how to get and manage
a single struct clk. Using devm_clk_get() would silently ignore the
second clock, so switch to devm_clk_bulk_get_all() and the matching
clk_bulk_prepare_enable()/clk_bulk_disable_unprepare() calls. This
lets the driver transparently handle however many clocks a given
compatible declares, without needing per-variant clock-handling code,
and keeps working for existing SoCs that only have one.

Fixes: 18d65de8157c ("thermal/drivers/amlogic: Add support for secure monitor calibration readout")
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
 drivers/thermal/amlogic_thermal.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
index a0b530624b60c..2e8d042067079 100644
--- a/drivers/thermal/amlogic_thermal.c
+++ b/drivers/thermal/amlogic_thermal.c
@@ -100,7 +100,8 @@ struct amlogic_thermal {
 	const struct amlogic_thermal_data *data;
 	struct regmap *regmap;
 	struct regmap *sec_ao_map;
-	struct clk *clk;
+	struct clk_bulk_data *clks;
+	int num_clks;
 	struct thermal_zone_device *tzd;
 	u32 trim_info;
 	struct meson_sm_firmware *sm_fw;
@@ -142,7 +143,7 @@ static int amlogic_thermal_enable(struct amlogic_thermal *data)
 {
 	int ret;
 
-	ret = clk_prepare_enable(data->clk);
+	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
 	if (ret)
 		return ret;
 
@@ -156,7 +157,7 @@ static void amlogic_thermal_disable(struct amlogic_thermal *data)
 {
 	regmap_update_bits(data->regmap, TSENSOR_CFG_REG1,
 			   TSENSOR_CFG_REG1_ENABLE, 0);
-	clk_disable_unprepare(data->clk);
+	clk_bulk_disable_unprepare(data->num_clks, data->clks);
 }
 
 static int amlogic_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
@@ -323,9 +324,10 @@ 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");
+	ret = devm_clk_bulk_get_all(dev, &pdata->clks);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to get clocks\n");
+	pdata->num_clks = ret;
 
 	if (pdata->data->use_sm)
 		ret = amlogic_thermal_probe_sm(pdev, pdata);

-- 
2.49.0

Re: [PATCH 2/2] thermal/drivers/amlogic: switch to clk_bulk API to support multiple clocks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] thermal/amlogic: kernel panic on unclocked register access if DT lacks clocks
--

commit ea5439a5f68769e10d58c93a949a283bf6984a5f
Author: Ronald Claveau <linux-kernel-dev@aliel.fr>

thermal/drivers/amlogic: switch to clk_bulk API to support multiple clocks

The t7-thermal variant now exposes two clocks (bus and core) in its
devicetree binding, but the driver only knows how to get and manage
a single struct clk. Using devm_clk_get() would silently ignore the
second clock, so switch to devm_clk_bulk_get_all() and the matching
clk_bulk_prepare_enable()/clk_bulk_disable_unprepare() calls.

> diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
> index a0b530624b60c..2e8d042067079 100644
> --- a/drivers/thermal/amlogic_thermal.c
> +++ b/drivers/thermal/amlogic_thermal.c
[ ... ]
> @@ -323,9 +324,10 @@ 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");
> +	ret = devm_clk_bulk_get_all(dev, &pdata->clks);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to get clocks\n");
> +	pdata->num_clks = ret;

[Severity: High]
Does this allow the probe to succeed without any clocks, leading to a crash
during register access if the device tree lacks clocks?

While device trees are typically trusted, missing required properties must
result in a graceful probe failure, not a system crash. The original
devm_clk_get() cleanly failed with -ENOENT if the clock was missing.

By changing this to devm_clk_bulk_get_all(), a missing clocks property now
returns 0. The driver sets pdata->num_clks = 0 and proceeds without error.

Later in probe, amlogic_thermal_enable() calls clk_bulk_prepare_enable()
(which becomes a no-op for 0 clocks) and then writes to TSENSOR_CFG_REG1.

Accessing hardware registers without first enabling the necessary bus or core
clocks usually results in a bus fault (synchronous external abort), causing a
kernel panic. Should there be an explicit check for ret == 0 to return an
error?

>  
>  	if (pdata->data->use_sm)
>  		ret = amlogic_thermal_probe_sm(pdev, pdata);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-fix-t7-thermal-clock-v1-0-7d4c4682f182@aliel.fr?part=2