Re: [RFC PATCH v3 03/37] dt-bindings: Add binding for kalray,kv3-1-intc

Krzysztof Kozlowski
2024-08-07T07:55:53Z

On 31/07/2024 16:47, Yann Sionneau wrote:
>>> +description: |
>>> + The Kalray Core Interrupt Controller is tightly integrated in each kv3 core
>>> + present in the Coolidge SoC.
>>> +
>>> + It provides the following features:
>>> + - 32 independent interrupt sources
>>> + - 2-bit configurable priority level
>>> + - 2-bit configurable ownership level
>>> +
>>> +properties:
>>> + compatible:
>>> + const: kalray,kv3-1-intc
>> What is the SoC name/model? You use "Kalray Core" and "Coolidge" and
>> "kv3-1".
>
> The SoC name is Coolidge , it contains "Kalray cores". Kalray being the name of the company.
>
> The exact core name is kv3-1. The core is part of the kvx family: next core generation will most likely be called kv4 or kv4-1.
>
> The question whether to use "coolidge" (soc name) or "kv3-1" (cpu core name) for our IPs compatible is indeed a good one, we talked about it internally at Kalray.
>
> We ended up using "kv3-1" at the beginning of the compatible when the hw IP is very closed/tightly integrated into the CPU core.
>
> We ended up using "coolidge" when the hw IP is just some general IP inside the SoC (like the IOMMU, the dma_noc).
>
> The "intc" or "core intc", is very tightly integrated into the cpu core.
>
> The "core intc" is configured directly using core registers named SFR (System Function Registers) using special SFR handling instructions. It is not memory mapped.
>
> So we ended up using "kv3-1" in the compatible. Is this OK?

Comaptibles are supposed to be based on SoC model name, thus kv3-1 is
fine (based in your explanation). However unified naming in title or
description would be helpful, e.g. in title:
"Kalray Coolidge kv3-1 Core Interrupt Controller"

And all other compatibles and titles need fixing. For example in other
places you use "coolidge".

>
>>
>>> +
>>> + "#interrupt-cells":
>>> + const: 1
>>> + description:
>>> + The IRQ number.
>>> +
>>> + "#address-cells":
>>> + const: 0
>>> +
>>> + interrupt-controller: true
>>> +
>>> +additionalProperties: false
>> Please put it after "required:" block. See example-schema. This applies
>> everywhere.
> Oops, ok, I will do this.
>>
>>> +
>>> +required:
>>> + - compatible
>>> + - "#interrupt-cells"
>>> + - "#address-cells"
>>> + - interrupt-controller
>>> +
>>> +examples:
>>> + - |
>>> + intc: interrupt-controller {
>> No resources? How does it talk with the hardware?
> CPU configures the core intc using special instructions to access SFR (System Function Registers).

Ack.

Best regards,
Krzysztof