diff mbox series

[v4,1/2] dt-bindings: interrupt-controller: add risc-v,aplic hart indexes

Message ID 20250109113814.3254448-2-vladimir.kondratiev@mobileye.com (mailing list archive)
State Superseded
Headers show
Series riscv,aplic: support for hart indexes | expand

Checks

Context Check Description
conchuod/vmtest-for-next-PR success PR summary
conchuod/patch-1-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh took 108.96s
conchuod/patch-1-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh took 1051.57s
conchuod/patch-1-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh took 1233.54s
conchuod/patch-1-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh took 16.52s
conchuod/patch-1-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh took 18.15s
conchuod/patch-1-test-6 success .github/scripts/patches/tests/checkpatch.sh took 0.38s
conchuod/patch-1-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh took 35.92s
conchuod/patch-1-test-8 success .github/scripts/patches/tests/header_inline.sh took 0.00s
conchuod/patch-1-test-9 success .github/scripts/patches/tests/kdoc.sh took 0.45s
conchuod/patch-1-test-10 success .github/scripts/patches/tests/module_param.sh took 0.01s
conchuod/patch-1-test-11 success .github/scripts/patches/tests/verify_fixes.sh took 0.00s
conchuod/patch-1-test-12 success .github/scripts/patches/tests/verify_signedoff.sh took 0.02s

Commit Message

Vladimir Kondratiev Jan. 9, 2025, 11:38 a.m. UTC
Document optional property "riscv,hart-indexes"

Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 .../bindings/interrupt-controller/riscv,aplic.yaml        | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Rob Herring (Arm) Jan. 10, 2025, 4:20 p.m. UTC | #1
On Thu, Jan 09, 2025 at 01:38:13PM +0200, Vladimir Kondratiev wrote:
> Document optional property "riscv,hart-indexes"
> 
> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
> Reviewed-by: Anup Patel <anup@brainfault.org>
> ---
>  .../bindings/interrupt-controller/riscv,aplic.yaml        | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> index 190a6499c932..bef00521d5da 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> @@ -91,6 +91,14 @@ properties:
>        Firmware must configure interrupt delegation registers based on
>        interrupt delegation list.
>  
> +  riscv,hart-indexes:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 16384
> +    description:
> +      A list of hart indexes that APLIC should use to address each hart
> +      that is mentioned in the "interrupts-extended"

Wouldn't using the 'cpus' property linking to each cpu/hart node work?

Rob
Rob Herring (Arm) Jan. 10, 2025, 4:22 p.m. UTC | #2
On Thu, Jan 09, 2025 at 01:38:13PM +0200, Vladimir Kondratiev wrote:
> Document optional property "riscv,hart-indexes"

That is obvious reading the diff. Why do you need this?

Also, what happens when this property is not present?

> 
> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
> Reviewed-by: Anup Patel <anup@brainfault.org>
> ---
>  .../bindings/interrupt-controller/riscv,aplic.yaml        | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> index 190a6499c932..bef00521d5da 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> @@ -91,6 +91,14 @@ properties:
>        Firmware must configure interrupt delegation registers based on
>        interrupt delegation list.
>  
> +  riscv,hart-indexes:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 16384
> +    description:
> +      A list of hart indexes that APLIC should use to address each hart
> +      that is mentioned in the "interrupts-extended"
> +
>  dependencies:
>    riscv,delegation: [ "riscv,children" ]
>  
> -- 
> 2.43.0
>
Vladimir Kondratiev Jan. 12, 2025, 7:47 a.m. UTC | #3
>Wouldn't using the 'cpus' property linking to each cpu/hart node work?

>Rob

Hi,

unfortunately, per-CPU property would not work. "hart index" defined per
interrupt domain, and different controllers may define it differently.
This is from the "4.3 Hart index numbers" section of the interrupts spec found at
https://github.com/riscv/riscv-aia

Within a given interrupt domain, each of the domain’s harts has a unique index
number in the range 0 to 2^14 − 1 (= 16,383).
Vladimir Kondratiev Jan. 12, 2025, 8:38 a.m. UTC | #4
>> Document optional property "riscv,hart-indexes"

>That is obvious reading the diff. Why do you need this?

I say it briefly in the description for the property.
In more details this is described in the other patch comment
- for code that uses this property.
Is it better to repeat more detailed description in this patch
comment as well?

>Also, what happens when this property is not present?
Logical hart index get used, i.e. index in the "extended-interrupts"

Shall I add full explanation to this patch comment? This one, it is
a comment from the 2-nd patch in this set:

Risc-V APLIC specification defines "hart index" in [1]:

Within a given interrupt domain, each of the domain’s harts has a
unique index number in the range 0 to 2^14 − 1 (= 16,383). The index
number a domain associates with a hart may or may not have any
relationship to the unique hart identifier (“hart ID”) that the
RISC-V Privileged Architecture assigns to the hart. Two different
interrupt domains may employ entirely different index numbers for
the same set of harts.

Further, this document says in "4.5 Memory-mapped control
region for an interrupt domain":

The array of IDC structures may include some for potential hart index
numbers that are not actual hart index numbers in the domain. For
example, the first IDC structure is always for hart index 0, but 0 is
not necessarily a valid index number for any hart in the domain.

Support arbitrary hart indexes specified in optional APLIC property
"riscv,hart-indexes" that should be array of u32 elements, one per
interrupt target. If this property not specified, fallback is to use
logical hart indexes within the domain.

[1]: https://github.com/riscv/riscv-aia


Thanks, Vladimir
Thomas Gleixner Jan. 27, 2025, 5:52 p.m. UTC | #5
On Sun, Jan 12 2025 at 08:38, Vladimir Kondratiev wrote:
>>> Document optional property "riscv,hart-indexes"
>
>>That is obvious reading the diff. Why do you need this?
>
> I say it briefly in the description for the property.
> In more details this is described in the other patch comment
> - for code that uses this property.
> Is it better to repeat more detailed description in this patch
> comment as well?

Obviously. Each patch has to be self contained and explain what it is
about.

Thanks,

        tglx
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
index 190a6499c932..bef00521d5da 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
@@ -91,6 +91,14 @@  properties:
       Firmware must configure interrupt delegation registers based on
       interrupt delegation list.
 
+  riscv,hart-indexes:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 16384
+    description:
+      A list of hart indexes that APLIC should use to address each hart
+      that is mentioned in the "interrupts-extended"
+
 dependencies:
   riscv,delegation: [ "riscv,children" ]