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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dt-bindings: ABI break by prepending new clock instead of appending
--

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

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

This commit updates the amlogic thermal binding to describe a new bus clock
required by the t7 thermal sensor to gate register access. It makes this
second clock mandatory for the t7-thermal compatible while keeping a single
clock for older Amlogic SoCs.

> diff --git a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
> --- 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.

[Severity: High]
Does prepending the "bus" clock at index 0 break backward compatibility for
older deployed kernels?

Older driver versions use devm_clk_get() which blindly fetches the clock at
index 0:

drivers/thermal/amlogic_thermal.c:amlogic_thermal_probe()
    ...
    pdata->clk = devm_clk_get(dev, NULL);
    if (IS_ERR(pdata->clk))
    ...

If an older kernel is booted with this updated device tree, it will fetch the
"bus" clock instead of the "core" functional clock, which could cause the
thermal sensor to fail. Additionally, doesn't this misdocument the existing
functional clock for older Amlogic SoCs as a "bus" clock?

Even if the driver is later updated to use bulk clock APIs, the device tree
ABI break for existing older kernels remains. Should the new "bus" clock be
appended to the list instead, keeping "core" at index 0 and "bus" at index 1?

> +
> +  clock-names:
> +    items:
> +      - const: bus
> +      - const: core

[Severity: High]
Since this naming array matches the clocks array above, would it also need to
be reversed to "- const: core" then "- const: bus" to ensure the functional
clock remains at index 0?

[ ... ]

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

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