Re: [RFC PATCH v2 02/31] Documentation: Add binding for kalray,kv3-1-core-intc

[Krzysztof Kozlowski]

On 20/01/2023 15:09, Yann Sionneau wrote:
> From: Jules Maselbas <jmaselbas@xxxxxxxxx>

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).


> 
> Add documentation for `kalray,kv3-1-core-intc` binding.
> 
> Co-developed-by: Jules Maselbas <jmaselbas@xxxxxxxxx>
> Signed-off-by: Jules Maselbas <jmaselbas@xxxxxxxxx>
> Signed-off-by: Yann Sionneau <ysionneau@xxxxxxxxx>
> ---
> 
> Notes:
>     V1 -> V2: new patch
> 
>  .../kalray,kv3-1-core-intc.yaml               | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-core-intc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-core-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-core-intc.yaml
> new file mode 100644
> index 000000000000..1e3d0593173a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-core-intc.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/kalray,kv3-1-core-intc#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Kalray kv3-1 Core Interrupt Controller
> +
> +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
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: kalray,kv3-1-core-intc

Blank line between each of these,

> +  "#interrupt-cells":
> +    const: 1
> +    description:
> +      The IRQ number.
> +  reg:
> +    maxItems: 0

??? No way... What's this?

> +  "kalray,intc-nr-irqs":

Drop quotes.

> +    description: Number of irqs handled by the controller.

Why this is variable per board? Why do you need it ?

> +
> +required:
> +  - compatible
> +  - "#interrupt-cells"
> +  - interrupt-controller

missing additionalProperties: false

This binding looks poor, like you started from something odd. Please
don't. Take the newest reviewed binding or better example-schema and use
it to build yours. This would solve several trivial mistakes and style
issues.

> +
> +examples:
> +  - |
> +    intc: interrupt-controller {

What's the IO address space?


Best regards,
Krzysztof