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

ysionneau@kalrayinc.com posted 37 patches 1 year, 1 month ago
[RFC PATCH v3 03/37] dt-bindings: Add binding for kalray,kv3-1-intc
Posted by ysionneau@kalrayinc.com 1 year, 1 month ago
From: Yann Sionneau <ysionneau@kalrayinc.com>

Add binding for Kalray kv3-1 core 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,kv3-1-intc.yaml                    | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml
new file mode 100644
index 0000000000000..9c8bb2c8c49dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/kalray,kv3-1-intc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Kalray kv3-1 Core Interrupt Controller
+
+maintainers:
+  - Jonathan Borne <jborne@kalrayinc.com>
+  - Julian Vetter <jvetter@kalrayinc.com>
+  - Yann Sionneau <ysionneau@kalrayinc.com>
+
+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
+
+  "#interrupt-cells":
+    const: 1
+    description:
+      The IRQ number.
+
+  "#address-cells":
+    const: 0
+
+  interrupt-controller: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - "#interrupt-cells"
+  - "#address-cells"
+  - interrupt-controller
+
+examples:
+  - |
+    intc: interrupt-controller {
+        compatible = "kalray,kv3-1-intc";
+        #interrupt-cells = <1>;
+        #address-cells = <0>;
+        interrupt-controller;
+    };
+
+...
-- 
2.45.2
Re: [RFC PATCH v3 03/37] dt-bindings: Add binding for kalray,kv3-1-intc
Posted by Rob Herring 1 year, 1 month ago
On Mon, Jul 22, 2024 at 11:41:14AM +0200, ysionneau@kalrayinc.com wrote:
> From: Yann Sionneau <ysionneau@kalrayinc.com>
> 
> Add binding for Kalray kv3-1 core 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,kv3-1-intc.yaml                    | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml
> new file mode 100644
> index 0000000000000..9c8bb2c8c49dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/kalray,kv3-1-intc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Kalray kv3-1 Core Interrupt Controller
> +
> +maintainers:
> +  - Jonathan Borne <jborne@kalrayinc.com>
> +  - Julian Vetter <jvetter@kalrayinc.com>
> +  - Yann Sionneau <ysionneau@kalrayinc.com>
> +
> +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

Why is this defined here and in the cpu schema? It is simple enough, I 
would just put it in the cpu schema.

Rob
Re: [RFC PATCH v3 03/37] dt-bindings: Add binding for kalray,kv3-1-intc
Posted by Krzysztof Kozlowski 1 year, 1 month ago
On 22/07/2024 11:41, ysionneau@kalrayinc.com wrote:
> From: Yann Sionneau <ysionneau@kalrayinc.com>
> 
> Add binding for Kalray kv3-1 core 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,kv3-1-intc.yaml                    | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml
> new file mode 100644
> index 0000000000000..9c8bb2c8c49dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/kalray,kv3-1-intc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Kalray kv3-1 Core Interrupt Controller
> +
> +maintainers:
> +  - Jonathan Borne <jborne@kalrayinc.com>
> +  - Julian Vetter <jvetter@kalrayinc.com>
> +  - Yann Sionneau <ysionneau@kalrayinc.com>
> +
> +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".

> +
> +  "#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.

> +
> +required:
> +  - compatible
> +  - "#interrupt-cells"
> +  - "#address-cells"
> +  - interrupt-controller
> +
> +examples:
> +  - |
> +    intc: interrupt-controller {

No resources? How does it talk with the hardware?

> +        compatible = "kalray,kv3-1-intc";
> +        #interrupt-cells = <1>;
> +        #address-cells = <0>;
> +        interrupt-controller;
> +    };
> +
> +...

Best regards,
Krzysztof
Re: [RFC PATCH v3 03/37] dt-bindings: Add binding for kalray,kv3-1-intc
Posted by Yann Sionneau 1 year, 1 month ago
Hello Krzysztof,

On 22/07/2024 11:51, Krzysztof Kozlowski wrote:
> On 22/07/2024 11:41, ysionneau@kalrayinc.com wrote:
>> From: Yann Sionneau <ysionneau@kalrayinc.com>
>>
>> Add binding for Kalray kv3-1 core 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,kv3-1-intc.yaml                    | 54 +++++++++++++++++++
>>  1 file changed, 54 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml
>> new file mode 100644
>> index 0000000000000..9c8bb2c8c49dd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-intc.yaml
>> @@ -0,0 +1,54 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/interrupt-controller/kalray,kv3-1-intc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Kalray kv3-1 Core Interrupt Controller
>> +
>> +maintainers:
>> +  - Jonathan Borne <jborne@kalrayinc.com>
>> +  - Julian Vetter <jvetter@kalrayinc.com>
>> +  - Yann Sionneau <ysionneau@kalrayinc.com>
>> +
>> +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?

>
>> +
>> +  "#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).
>
>> +        compatible = "kalray,kv3-1-intc";
>> +        #interrupt-cells = <1>;
>> +        #address-cells = <0>;
>> +        interrupt-controller;
>> +    };
>> +
>> +...

Regards,

-- 

Yann
Re: [RFC PATCH v3 03/37] dt-bindings: Add binding for kalray,kv3-1-intc
Posted by Krzysztof Kozlowski 1 year, 1 month ago
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