Re: [RFC PATCH v3 05/37] dt-bindings: Add binding for kalray,coolidge-apic-mailbox

Yann Sionneau
2024-09-04T15:10:03Z

Hello Rob,

Le 7/22/24 à 22:47, Rob Herring a écrit :
> On Mon, Jul 22, 2024 at 11:41:16AM +0200, ysionneau@kalrayinc.com wrote:
>> From: Yann Sionneau <ysionneau@kalrayinc.com>
>>
>> Add binding for Kalray Coolidge APIC Mailbox interrupt-controller.
>>
>> Co-developed-by: Jules Maselbas <jmaselbas@zdiv.net>
>> Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
>> Signed-off-by: Yann Sionneau <ysionneau@kalrayinc.com>
>> ---
>>
>> Notes:
>>
>> V2 -> V3: Fixed bindings to adhere to dt-schema
>> ---
>> .../kalray,coolidge-apic-mailbox.yaml | 90 +++++++++++++++++++
>> 1 file changed, 90 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kalray,coolidge-apic-mailbox.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/kalray,coolidge-apic-mailbox.yaml b/Documentation/devicetree/bindings/interrupt-controller/kalray,coolidge-apic-mailbox.yaml
>> new file mode 100644
>> index 0000000000000..334b816b80583
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/kalray,coolidge-apic-mailbox.yaml
>> [...]
>> +
>> + interrupts:
>> + maxItems: 128
>> + minItems: 1
>> + description: |
>> + Specifies the interrupt line(s) in the interrupt-parent controller node;
>> + valid values depend on the type of parent interrupt controller
> Your description applies to all 'interrupts' properties and is therefore
> redundant. What you should explain is what are the 1-128 possible
> interrupts. Normally, you have to list each one out unless they are all
> instances of the same type of interrupt.

Oops I understand what you mean, this description could work on any .yaml indeed.

I propose this instead:

Each of the 128 mailboxes generates an interrupt on the APIC-GIC.

The interrupt is triggered according to the mailbox input mode and

its trigger condition.

>> +
>> + msi-controller: true
> "#msi-cells" should be specified too.
Ok.
>
>> +
>> +additionalProperties: false
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - "#interrupt-cells"
>> + - "#address-cells"
>> + - interrupt-controller
>> + - interrupts
>> + - msi-controller
>> +
>> +examples:
>> + - |
>> + apic_mailbox: interrupt-controller@a00000 {
>> + compatible = "kalray,coolidge-apic-mailbox";
>> + reg = <0 0xa00000 0 0x0f200>;
>> + #interrupt-cells = <0>;
>> + interrupt-controller;
>> + interrupt-parent = <&apic_gic>;
>> + interrupts = <0>, <1>, <2>, <3>, <4>, <5>, <6>, <7>, <8>, <9>;
>> + msi-controller;
>> + };
>> +
>> +...
>> --
>> 2.45.2
>>
>>
>>
>>
>>
>