Re: [PATCH] dt-binding: thermal: Convert marvell,armada-ap806-thermal to DT schema

[Miquel Raynal <miquel.raynal@bootlin.com>]

On 01/08/2025 at 15:22:20 -05, Rob Herring <robh@kernel.org> wrote:

> On Fri, Aug 1, 2025 at 9:27 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>>
>> Hi Rob,
>>
>> Sorry for the delay, I don't know why I forgot these.
>>
>> ...
>>
>> > +properties:
>> > +  compatible:
>> > +    enum:
>> > +      - marvell,armada-ap806-thermal
>> > +      - marvell,armada-ap807-thermal
>> > +      - marvell,armada-cp110-thermal
>> > +
>> > +  reg:
>> > +    maxItems: 1
>> > +
>> > +  interrupts:
>> > +    description: overheat interrupt
>> > +    maxItems: 1
>> > +
>> > +  '#thermal-sensor-cells':
>> > +    description: Cell represents the channel ID. There is one sensor per
>> > +      channel. O refers to the thermal IP internal channel.
>> > +    const: 1
>> > +
>> > +required:
>> > +  - compatible
>> > +  - reg
>> > +
>> > +additionalProperties: false
>>
>> IIRC on these Marvell designs, there was one (or more, I don't remember)
>> Application Processor (AP) and several Co-Processors (CP).
>>
>> [On the AP]
>> The AP8XX overheat interrupt was not directly wired to the GIC but was
>> going through another intermediate IRQ controller named SEI (System
>> Error Interrupt).
>>
>>       Thermal overheat IRQ -> SEI -> GIC
>>
>> [On the CP]
>> There was one interrupt controller per CP11X named ICU, which would be
>> connected to the top level GIC through MSIs. The ICU was however split
>> into several sub-controllers reaching different areas on the GIC.
>>
>>                                       MSI
>>       Thermal overheat IRQ -> ICU SEI -> GIC
>>
>> As the OS could not guess the internal connexions, I believe we had to
>> include in the bindings the parent IRQ chip we were connected to. In the
>> case of the thermal over heat interrupts, they were all going through an
>> SEI controller (System Error Interrupt) which, if I still remember
>> correctly, was not the default parent, hence the use of
>> interrupts-parent/interrupts-extended in the examples.
>>
>> This is all a bit cloudy in my mind, but I believe these properties
>> matter and with 'additionalProperties: false' and without
>> interrupts-parent/interrupts-extended allowed, a real world DT
>> snippet would not pass the binding checks.
>
> 'interrupt-parent' is implicitly allowed anywhere. Who is the parent
> is outside the scope of the binding given it can vary.

Ok, good to know.

>> > +examples:
>> > +  - |
>> > +    thermal-sensor@80 {
>> > +        compatible = "marvell,armada-ap806-thermal";
>> > +        reg = <0x80 0x10>;
>> > +        interrupts = <18>;
>>
>> I do not know how accurate the example must be, but maybe the example
>> shall reflect the SEI connection as well.
>
> No one has cared about converting the Marvell bindings, so *shrug*.

I was still mentioning the same interrupt-parent property here ("the SEI
connection"), not how accurate the values were. So if we do not care
about interrupt-parent because it is allowed anyway,

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks for the conversion,
Miquèl