Re: [RFC PATCH v2 02/31] Documentation: Add binding for kalray,kv3-1-core-intc
Hi Krzysztof,
On Sun, Jan 22, 2023 at 12:44:46PM +0100, Krzysztof Kozlowski wrote:
On 20/01/2023 15:09, Yann Sionneau wrote:
> From: Jules Maselbas <jmaselbas(a)kalray.eu>
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).
This will be fixed, sorry for the inconvenience.
>
> Add documentation for `kalray,kv3-1-core-intc` binding.
>
> Co-developed-by: Jules Maselbas <jmaselbas(a)kalray.eu>
> Signed-off-by: Jules Maselbas <jmaselbas(a)kalray.eu>
> Signed-off-by: Yann Sionneau <ysionneau(a)kalray.eu>
> ---
>
> 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,
Ack
> + "#interrupt-cells":
> + const: 1
> + description:
> + The IRQ number.
> + reg:
> + maxItems: 0
??? No way... What's this?
This (per CPU) interrupt controller is not memory
mapped at all, it is
controlled and configured through system registers.
I do not have found existing .yaml bindings for such devices, only the
file snps,archs-intc.txt has something similar.
I do not know what is the best way to represent such devices in the
device-tree. Any suggestions are welcome.
> + "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 ?
This property is not even used
in our device-tree, this will be removed
from the documentation and from the driver as well.
> +
> +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.
I am starting over from the example-schema.
> +
> +examples:
> + - |
> + intc: interrupt-controller {
What's the IO address space?
As said above, this is not a memory mapped device,
but is accessed
through system registers.
Thanks,
-- Jules