From: Anup Patel <apatel@ventanamicro.com> To: Conor Dooley <conor@kernel.org> Cc: Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, Thomas Gleixner <tglx@linutronix.de>, Marc Zyngier <maz@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Atish Patra <atishp@atishpatra.org>, Alistair Francis <Alistair.Francis@wdc.com>, Anup Patel <anup@brainfault.org>, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH v2 6/9] dt-bindings: interrupt-controller: Add RISC-V advanced PLIC Date: Mon, 20 Feb 2023 10:06:49 +0530 [thread overview] Message-ID: <CAK9=C2UPa5SATTG1pJdckqaoJxh-8EBz8xsENirZQ-vbaHTgjg@mail.gmail.com> (raw) In-Reply-To: <Y7X62v5Zp6+thx5A@spud> On Thu, Jan 5, 2023 at 3:47 AM Conor Dooley <conor@kernel.org> wrote: > > Hey Anup, > > On Tue, Jan 03, 2023 at 07:44:06PM +0530, Anup Patel wrote: > > We add DT bindings document for RISC-V advanced platform level > > interrupt controller (APLIC) defined by the RISC-V advanced > > interrupt architecture (AIA) specification. > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > --- > > .../interrupt-controller/riscv,aplic.yaml | 159 ++++++++++++++++++ > > 1 file changed, 159 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml > > > + interrupts-extended: > > + minItems: 1 > > + maxItems: 16384 > > + description: > > + Given APLIC domain directly injects external interrupts to a set of > > + RISC-V HARTS (or CPUs). Each node pointed to should be a riscv,cpu-intc > > + node, which has a riscv node (i.e. RISC-V HART) as parent. > > + > > + msi-parent: > > + description: > > + Given APLIC domain forwards wired interrupts as MSIs to a AIA incoming > > + message signaled interrupt controller (IMSIC). This property should be > > + considered only when the interrupts-extended property is absent. > > Considered by what? > On v1 you said: > <quote> > If both "interrupts-extended" and "msi-parent" are present then it means > the APLIC domain supports both MSI mode and Direct mode in HW. In this > case, the APLIC driver has to choose between MSI mode or Direct mode. > <\quote> > > The description is still pretty ambiguous IMO. Perhaps incorporate > some of that expanded comment into the property description? > Say, "If both foo and bar are present, the APLIC domain has hardware > support for both MSI and direct mode. Software may then chose either > mode". > Have I misunderstood your comment on v1? It read as if having both > present indicated that both were possible & that "should be considered > only..." was more of a suggestion and a comment about the Linux driver's > behaviour. > Apologies if I have misunderstood, but I suppose if I have then the > binding's description could be improved!! Yes, when both DT properties are present then it's up to Linux APLIC driver to choose the appropriate APLIC mode. I forgot to update the text here in v2 but I will update it in v3. Thanks for pointing. > > > + riscv,children: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + minItems: 1 > > + maxItems: 1024 > > + items: > > + maxItems: 1 > > + description: > > + A list of child APLIC domains for the given APLIC domain. Each child > > + APLIC domain is assigned child index in increasing order with the > > btw, missing article before child (& a comma after order I think). Okay, I will update. > > > + first child APLIC domain assigned child index 0. The APLIC domain > > + child index is used by firmware to delegate interrupts from the > > + given APLIC domain to a particular child APLIC domain. > > + > > + riscv,delegate: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + minItems: 1 > > + maxItems: 1024 > > Is it valid to have a delegate property without children? If not, the > binding should reflect that dependency IMO. Okay, I will update. > > > + items: > > + items: > > + - description: child APLIC domain phandle > > + - description: first interrupt number (inclusive) > > + - description: last interrupt number (inclusive) > > + description: > > + A interrupt delegation list where each entry is a triple consisting > > + of child APLIC domain phandle, first interrupt number, and last > > + interrupt number. The firmware will configure interrupt delegation > > btw, drop the article before firmware here. > Also, "firmware will" or "firmware must"? Semantics perhaps, but they > are different! I think "firmware must" is better because APLIC M-mode domains are not accessible to S-mode so firmware must configure delegation for at least all APLIC M-mode domains. > > Kinda for my own curiosity here, but do you expect these properties to > generally be dynamically filled in by the bootloader or read by the > bootloader to set up the configuration? Firmware (or bootloader) will look at this property and setup delegation before booting the OS kernel. > > > + registers based on interrupt delegation list. > > I'm sorry Anup, but this child versus delegate thing is still not clear > to me binding wise. See below. There are two different information in-context of APLIC domain: 1) HW child domain numbering: If an APLIC domain has N children then HW will have a fixed child index for each of the N children in the range 0 to N-1. This HW child index is required at the time of setting up interrupt delegation in sourcecfgX registers. The "riscv,children" DT property helps firmware (or bootloader) find the total number of child APLIC domains and corresponding HW child index number. 2) IRQ delegation to child domains: An APLIC domain can delegate any IRQ range(s) to a particular APLIC child domain. The "riscv,delegate" DT property is simply a table where we have one row for each IRQ range which is delegated to some child APLIC domain. This property is more of a system setting fixed by the RISC-V platform vendor. > > > + aplic0: interrupt-controller@c000000 { > > + compatible = "riscv,qemu-aplic", "riscv,aplic"; > > + interrupts-extended = <&cpu1_intc 11>, > > + <&cpu2_intc 11>, > > + <&cpu3_intc 11>, > > + <&cpu4_intc 11>; > > + reg = <0xc000000 0x4080>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + riscv,num-sources = <63>; > > + riscv,children = <&aplic1>, <&aplic2>; > > + riscv,delegate = <&aplic1 1 63>; > > Is aplic2 here for demonstrative purposes only, since it has not been > delegated any interrupts? Yes, it's for demonstrative purposes only. > I suppose it is hardware present on the SoC that is not being used by > the current configuration? Yes, in this example aplic2 is unused because it has no interrupts delegated to it. > > Thanks, > Conor. > > > + }; > > + > > + aplic1: interrupt-controller@d000000 { > > + compatible = "riscv,qemu-aplic", "riscv,aplic"; > > + interrupts-extended = <&cpu1_intc 9>, > > + <&cpu2_intc 9>; > > + reg = <0xd000000 0x4080>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + riscv,num-sources = <63>; > > + }; > > + > > + aplic2: interrupt-controller@e000000 { > > + compatible = "riscv,qemu-aplic", "riscv,aplic"; > > + interrupts-extended = <&cpu3_intc 9>, > > + <&cpu4_intc 9>; > > + reg = <0xe000000 0x4080>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + riscv,num-sources = <63>; > > + }; > Regards, Anup
WARNING: multiple messages have this Message-ID (diff)
From: Anup Patel <apatel@ventanamicro.com> To: Conor Dooley <conor@kernel.org> Cc: Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, Thomas Gleixner <tglx@linutronix.de>, Marc Zyngier <maz@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Atish Patra <atishp@atishpatra.org>, Alistair Francis <Alistair.Francis@wdc.com>, Anup Patel <anup@brainfault.org>, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH v2 6/9] dt-bindings: interrupt-controller: Add RISC-V advanced PLIC Date: Mon, 20 Feb 2023 10:06:49 +0530 [thread overview] Message-ID: <CAK9=C2UPa5SATTG1pJdckqaoJxh-8EBz8xsENirZQ-vbaHTgjg@mail.gmail.com> (raw) In-Reply-To: <Y7X62v5Zp6+thx5A@spud> On Thu, Jan 5, 2023 at 3:47 AM Conor Dooley <conor@kernel.org> wrote: > > Hey Anup, > > On Tue, Jan 03, 2023 at 07:44:06PM +0530, Anup Patel wrote: > > We add DT bindings document for RISC-V advanced platform level > > interrupt controller (APLIC) defined by the RISC-V advanced > > interrupt architecture (AIA) specification. > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > --- > > .../interrupt-controller/riscv,aplic.yaml | 159 ++++++++++++++++++ > > 1 file changed, 159 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml > > > + interrupts-extended: > > + minItems: 1 > > + maxItems: 16384 > > + description: > > + Given APLIC domain directly injects external interrupts to a set of > > + RISC-V HARTS (or CPUs). Each node pointed to should be a riscv,cpu-intc > > + node, which has a riscv node (i.e. RISC-V HART) as parent. > > + > > + msi-parent: > > + description: > > + Given APLIC domain forwards wired interrupts as MSIs to a AIA incoming > > + message signaled interrupt controller (IMSIC). This property should be > > + considered only when the interrupts-extended property is absent. > > Considered by what? > On v1 you said: > <quote> > If both "interrupts-extended" and "msi-parent" are present then it means > the APLIC domain supports both MSI mode and Direct mode in HW. In this > case, the APLIC driver has to choose between MSI mode or Direct mode. > <\quote> > > The description is still pretty ambiguous IMO. Perhaps incorporate > some of that expanded comment into the property description? > Say, "If both foo and bar are present, the APLIC domain has hardware > support for both MSI and direct mode. Software may then chose either > mode". > Have I misunderstood your comment on v1? It read as if having both > present indicated that both were possible & that "should be considered > only..." was more of a suggestion and a comment about the Linux driver's > behaviour. > Apologies if I have misunderstood, but I suppose if I have then the > binding's description could be improved!! Yes, when both DT properties are present then it's up to Linux APLIC driver to choose the appropriate APLIC mode. I forgot to update the text here in v2 but I will update it in v3. Thanks for pointing. > > > + riscv,children: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + minItems: 1 > > + maxItems: 1024 > > + items: > > + maxItems: 1 > > + description: > > + A list of child APLIC domains for the given APLIC domain. Each child > > + APLIC domain is assigned child index in increasing order with the > > btw, missing article before child (& a comma after order I think). Okay, I will update. > > > + first child APLIC domain assigned child index 0. The APLIC domain > > + child index is used by firmware to delegate interrupts from the > > + given APLIC domain to a particular child APLIC domain. > > + > > + riscv,delegate: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + minItems: 1 > > + maxItems: 1024 > > Is it valid to have a delegate property without children? If not, the > binding should reflect that dependency IMO. Okay, I will update. > > > + items: > > + items: > > + - description: child APLIC domain phandle > > + - description: first interrupt number (inclusive) > > + - description: last interrupt number (inclusive) > > + description: > > + A interrupt delegation list where each entry is a triple consisting > > + of child APLIC domain phandle, first interrupt number, and last > > + interrupt number. The firmware will configure interrupt delegation > > btw, drop the article before firmware here. > Also, "firmware will" or "firmware must"? Semantics perhaps, but they > are different! I think "firmware must" is better because APLIC M-mode domains are not accessible to S-mode so firmware must configure delegation for at least all APLIC M-mode domains. > > Kinda for my own curiosity here, but do you expect these properties to > generally be dynamically filled in by the bootloader or read by the > bootloader to set up the configuration? Firmware (or bootloader) will look at this property and setup delegation before booting the OS kernel. > > > + registers based on interrupt delegation list. > > I'm sorry Anup, but this child versus delegate thing is still not clear > to me binding wise. See below. There are two different information in-context of APLIC domain: 1) HW child domain numbering: If an APLIC domain has N children then HW will have a fixed child index for each of the N children in the range 0 to N-1. This HW child index is required at the time of setting up interrupt delegation in sourcecfgX registers. The "riscv,children" DT property helps firmware (or bootloader) find the total number of child APLIC domains and corresponding HW child index number. 2) IRQ delegation to child domains: An APLIC domain can delegate any IRQ range(s) to a particular APLIC child domain. The "riscv,delegate" DT property is simply a table where we have one row for each IRQ range which is delegated to some child APLIC domain. This property is more of a system setting fixed by the RISC-V platform vendor. > > > + aplic0: interrupt-controller@c000000 { > > + compatible = "riscv,qemu-aplic", "riscv,aplic"; > > + interrupts-extended = <&cpu1_intc 11>, > > + <&cpu2_intc 11>, > > + <&cpu3_intc 11>, > > + <&cpu4_intc 11>; > > + reg = <0xc000000 0x4080>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + riscv,num-sources = <63>; > > + riscv,children = <&aplic1>, <&aplic2>; > > + riscv,delegate = <&aplic1 1 63>; > > Is aplic2 here for demonstrative purposes only, since it has not been > delegated any interrupts? Yes, it's for demonstrative purposes only. > I suppose it is hardware present on the SoC that is not being used by > the current configuration? Yes, in this example aplic2 is unused because it has no interrupts delegated to it. > > Thanks, > Conor. > > > + }; > > + > > + aplic1: interrupt-controller@d000000 { > > + compatible = "riscv,qemu-aplic", "riscv,aplic"; > > + interrupts-extended = <&cpu1_intc 9>, > > + <&cpu2_intc 9>; > > + reg = <0xd000000 0x4080>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + riscv,num-sources = <63>; > > + }; > > + > > + aplic2: interrupt-controller@e000000 { > > + compatible = "riscv,qemu-aplic", "riscv,aplic"; > > + interrupts-extended = <&cpu3_intc 9>, > > + <&cpu4_intc 9>; > > + reg = <0xe000000 0x4080>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + riscv,num-sources = <63>; > > + }; > Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2023-02-20 4:37 UTC|newest] Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-01-03 14:14 [PATCH v2 0/9] Linux RISC-V AIA Support Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 1/9] RISC-V: Add AIA related CSR defines Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-04 23:07 ` Conor Dooley 2023-01-04 23:07 ` Conor Dooley 2023-01-09 5:09 ` Anup Patel 2023-01-09 5:09 ` Anup Patel 2023-01-17 20:42 ` Conor Dooley 2023-01-17 20:42 ` Conor Dooley 2023-01-27 11:58 ` Anup Patel 2023-01-27 11:58 ` Anup Patel 2023-01-27 14:20 ` Conor Dooley 2023-01-27 14:20 ` Conor Dooley 2023-01-03 14:14 ` [PATCH v2 2/9] RISC-V: Detect AIA CSRs from ISA string Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 3/9] irqchip/riscv-intc: Add support for RISC-V AIA Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-13 9:39 ` Marc Zyngier 2023-01-13 9:39 ` Marc Zyngier 2023-01-03 14:14 ` [PATCH v2 4/9] dt-bindings: interrupt-controller: Add RISC-V incoming MSI controller Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-04 23:21 ` Conor Dooley 2023-01-04 23:21 ` Conor Dooley 2023-02-20 3:15 ` Anup Patel 2023-02-20 3:15 ` Anup Patel 2023-01-12 20:49 ` Rob Herring 2023-01-12 20:49 ` Rob Herring 2023-02-20 3:20 ` Anup Patel 2023-02-20 3:20 ` Anup Patel 2023-02-19 11:17 ` Vivian Wang 2023-02-19 11:17 ` Vivian Wang 2023-02-20 3:31 ` Anup Patel 2023-02-20 3:31 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 5/9] irqchip: Add RISC-V incoming MSI controller driver Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-13 10:10 ` Marc Zyngier 2023-01-13 10:10 ` Marc Zyngier 2023-05-01 8:28 ` Anup Patel 2023-05-01 8:28 ` Anup Patel 2023-05-01 8:44 ` Marc Zyngier 2023-05-01 8:44 ` Marc Zyngier [not found] ` <CAPqJEFqhd-=-RYepKqnco7HySoxk7AhEctL+vzNozMSWe0mv7A@mail.gmail.com> [not found] ` <CABvJ_xhcuC92A_oo1mWQoRvtRzE8XXx9bbXKs7N7wKm0=Z3_Cw@mail.gmail.com> 2023-01-18 3:49 ` Fwd: " Vincent Chen 2023-01-18 3:49 ` Vincent Chen 2023-01-18 4:20 ` Anup Patel 2023-01-18 4:20 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 6/9] dt-bindings: interrupt-controller: Add RISC-V advanced PLIC Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-04 22:16 ` Conor Dooley 2023-01-04 22:16 ` Conor Dooley 2023-02-20 4:36 ` Anup Patel [this message] 2023-02-20 4:36 ` Anup Patel 2023-02-20 10:32 ` Conor Dooley 2023-02-20 10:32 ` Conor Dooley 2023-02-20 10:56 ` Conor Dooley 2023-02-20 10:56 ` Conor Dooley 2023-01-12 21:02 ` Rob Herring 2023-01-12 21:02 ` Rob Herring 2023-02-19 11:48 ` Vivian Wang 2023-02-19 11:48 ` Vivian Wang 2023-02-20 5:09 ` Anup Patel 2023-02-20 5:09 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 7/9] irqchip: Add RISC-V advanced PLIC driver Anup Patel 2023-01-03 14:14 ` Anup Patel [not found] ` <CAPqJEFpmAvWiOdackxYwSPBfjo4DnTHXrXVSCC4snMn8tnZXPw@mail.gmail.com> [not found] ` <CABvJ_xhjMa8xTsO-Qa23TOqxPpYxyBYSfV6TmKney-Gp3oi8cA@mail.gmail.com> 2023-01-17 7:09 ` Fwd: " Vincent Chen 2023-01-17 7:09 ` Vincent Chen 2023-01-18 4:37 ` Anup Patel 2023-01-18 4:37 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 8/9] RISC-V: Select APLIC and IMSIC drivers Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 9/9] MAINTAINERS: Add entry for RISC-V AIA drivers Anup Patel 2023-01-03 14:14 ` Anup Patel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAK9=C2UPa5SATTG1pJdckqaoJxh-8EBz8xsENirZQ-vbaHTgjg@mail.gmail.com' \ --to=apatel@ventanamicro.com \ --cc=Alistair.Francis@wdc.com \ --cc=anup@brainfault.org \ --cc=atishp@atishpatra.org \ --cc=conor@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=maz@kernel.org \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ --cc=robh+dt@kernel.org \ --cc=tglx@linutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.