Re: [PATCH 1/8] dt-bindings: thermal: amlogic: Add support for T7

[Ronald Claveau]

Hello Krzysztof, thank you for this feedback.

On 4/12/26 11:58 AM, Krzysztof Kozlowski wrote:
> On Fri, Apr 10, 2026 at 06:48:02PM +0200, Ronald Claveau wrote:
>> Add the amlogic,t7-thermal compatible for the Amlogic T7 thermal sensor.
>>
>> Unlike existing variants which use a phandle to the ao-secure syscon,
>> the T7 relies on a secure monitor interface described by a phandle and
>> a sensor index argument.
>>
>> Introduce the amlogic,secure-monitor property as a phandle-array and
>> make amlogic,ao-secure or amlogic,secure-monitor conditionally required
>> depending on the compatible.
>>
>> Signed-off-by: Ronald Claveau <linux-kernel-dev@xxxxxxxx>
>> ---
>>  .../bindings/thermal/amlogic,thermal.yaml          | 40 +++++++++++++++++++++-
>>  1 file changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
>> index 70b273271754b..85ee73c6e1161 100644
>> --- a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
>> +++ b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
>> @@ -22,6 +22,7 @@ properties:
>>                - amlogic,g12a-ddr-thermal
>>            - const: amlogic,g12a-thermal
>>        - const: amlogic,a1-cpu-thermal
>> +      - const: amlogic,t7-thermal
> 
> So these two entries are enum.
> 

I will change to enum.

>>  
>>    reg:
>>      maxItems: 1
>> @@ -42,12 +43,40 @@ properties:
>>    '#thermal-sensor-cells':
>>      const: 0
>>  
>> +  amlogic,secure-monitor:
>> +    description: phandle to the secure monitor
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    items:
>> +      - items:
>> +          - description: phandle to the secure monitor
>> +          - description: sensor index
> 
> For what exactly this sensor index is needed? commit msg explained me
> nothing, instead repeated what you did. That's pointless, explain why
> you did it.
> 

Thanks I will add the explanation in the commit message and in the
description here.

>> +
>>  required:
>>    - compatible
>>    - reg
>>    - interrupts
>>    - clocks
>> -  - amlogic,ao-secure
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - amlogic,g12a-cpu-thermal
>> +              - amlogic,g12a-ddr-thermal
> 
> Drop both, you need only fallback.
> 

I hesitated between the two methods. I will change to fallback only.

>> +              - amlogic,a1-cpu-thermal
> 
> And list is sorted alphabetically.
> 

Thanks for the reminder.

>> +    then:
>> +      required:
>> +        - amlogic,ao-secure
> 
> Best regards,
> Krzysztof
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-amlogic


-- 
Best regards,
Ronald