| Message ID | 20180804082319.5711-7-hch@lst.de |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | None | expand |
On Sat, Aug 4, 2018 at 2:23 AM Christoph Hellwig <hch@lst.de> wrote: > > From: Palmer Dabbelt <palmer@dabbelt.com> Version numbers on the individual patches would be nice... > This patch adds documentation for the platform-level interrupt > controller (PLIC) found in all RISC-V systems. This interrupt > controller routes interrupts from all the devices in the system to each > hart-local interrupt controller. > > Note: the DTS bindings for the PLIC aren't set in stone yet, as we might > want to change how we're specifying holes in the hart list. > > Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com> > [hch: various fixes and updates] > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > .../interrupt-controller/sifive,plic0.txt | 57 +++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt > new file mode 100644 > index 000000000000..bbfa61cf8d3f > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt > @@ -0,0 +1,57 @@ > +SiFive Platform-Level Interrupt Controller (PLIC) > +------------------------------------------------- > + > +SiFive SOCs include an implementation of the Platform-Level Interrupt Controller > +(PLIC) high-level specification in the RISC-V Privileged Architecture > +specification. The PLIC connects all external interrupts in the system to all > +hart contexts in the system, via the external interrupt source in each hart. > + > +A hart context is a privilege mode in a hardware execution thread. For example, > +in an 4 core system with 2-way SMT, you have 8 harts and probably at least two > +privilege modes per hart; machine mode and supervisor mode. > + > +Each interrupt can be enabled on per-context basis. Any context can claim > +a pending enabled interrupt and then release it once it has been handled. > + > +Each interrupt has a configurable priority. Higher priority interrupts are > +serviced first. Each context can specify a priority threshold. Interrupts > +with priority below this threshold will not cause the PLIC to raise its > +interrupt line leading to the context. > + > +While the PLIC supports both edge-triggered and level-triggered interrupts, > +interrupt handlers are oblivious to this distinction and therefore it is not > +specified in the PLIC device-tree binding. > + > +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the > +"sifive,plic0" device is a concrete implementation of the PLIC that contains a > +specific memory layout, which is documented in chapter 8 of the SiFive U5 > +Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>. > + > +Required properties: > +- compatible : "sifive,plic0". > +- #address-cells : should be <0>. > +- #interrupt-cells : should be <1>. > +- interrupt-controller : Identifies the node as an interrupt controller. > +- reg : Should contain 1 register range (address and length). > +- interrupts-extended : Specifies which contexts are connected to the PLIC, > + with "-1" specifying that a context is not present. The nodes pointed > + to should be "riscv" HART nodes, or eventually be parented by such nodes. > +- riscv,ndev: Specifies how many external interrupts are supported by > + this controller. > + > +Example: > + > + plic: interrupt-controller@c000000 { > + #address-cells = <0>; > + #interrupt-cells = <1>; > + compatible = "riscv,plic0"; > + interrupt-controller; > + interrupts-extended = < > + &cpu0-intc 11 > + &cpu1-intc 11 &cpu1-intc 9 > + &cpu2-intc 11 &cpu2-intc 9 > + &cpu3-intc 11 &cpu3-intc 9 > + &cpu4-intc 11 &cpu4-intc 9>; I'm confused why this is still here if you are dropping the cpu intc binding? I also noticed the cpu binding refers to "riscv,cpu-intc" as well. That needs to be fixed too if there's a change. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 08, 2018 at 08:29:50AM -0600, Rob Herring wrote: > Version numbers on the individual patches would be nice... We've never done these in the subsystems I'm involved with. Too much clutter in the subject lines for information that is easily deductable. > > +Example: > > + > > + plic: interrupt-controller@c000000 { > > + #address-cells = <0>; > > + #interrupt-cells = <1>; > > + compatible = "riscv,plic0"; > > + interrupt-controller; > > + interrupts-extended = < > > + &cpu0-intc 11 > > + &cpu1-intc 11 &cpu1-intc 9 > > + &cpu2-intc 11 &cpu2-intc 9 > > + &cpu3-intc 11 &cpu3-intc 9 > > + &cpu4-intc 11 &cpu4-intc 9>; > > I'm confused why this is still here if you are dropping the cpu intc binding? We need some parent that identifies the core (hart in RISC-V terminology). The way the code now works is that it just walks up the parent chain until it finds a CPU node, so it either accepts the legacy intc node inbetween, or it accepts the cpu node directly as the intc node is pointless. I guess for the documentation we should instead just point to the "riscv" cpu nodes instead? > I also noticed the cpu binding refers to "riscv,cpu-intc" as well. > That needs to be fixed too if there's a change. Only in the examples. I'd be fine with dropping them, but let's keep that separate from the interrupt support. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 8, 2018 at 8:59 AM Christoph Hellwig <hch@lst.de> wrote: > > On Wed, Aug 08, 2018 at 08:29:50AM -0600, Rob Herring wrote: > > Version numbers on the individual patches would be nice... > > We've never done these in the subsystems I'm involved with. Too > much clutter in the subject lines for information that is easily > deductable. Unfortunately not in Gmail which doesn't thread properly. But patchwork also provides the version tag which I use to sort my reviews. > > > +Example: > > > + > > > + plic: interrupt-controller@c000000 { > > > + #address-cells = <0>; > > > + #interrupt-cells = <1>; > > > + compatible = "riscv,plic0"; > > > + interrupt-controller; > > > + interrupts-extended = < > > > + &cpu0-intc 11 > > > + &cpu1-intc 11 &cpu1-intc 9 > > > + &cpu2-intc 11 &cpu2-intc 9 > > > + &cpu3-intc 11 &cpu3-intc 9 > > > + &cpu4-intc 11 &cpu4-intc 9>; > > > > I'm confused why this is still here if you are dropping the cpu intc binding? > > We need some parent that identifies the core (hart in RISC-V terminology). > The way the code now works is that it just walks up the parent chain > until it finds a CPU node, so it either accepts the legacy intc node > inbetween, or it accepts the cpu node directly as the intc node is pointless. > > I guess for the documentation we should instead just point to the > "riscv" cpu nodes instead? That's not valid and dtc will tell you that. 'interrupts' (via interrupt-parent) or 'interrupts-extended' has to point to an 'interrupt-controller' node. I guess you could make the cpu nodes interrupt-controllers. That's a bit strange, but I can't think of a reason why that wouldn't work. Just because the cpu-intc is not made to be an irqchip in the kernel doesn't mean it can't still be represented as an interrupt-controller in DT. It shouldn't be designed just around how some OS happens to implement things. > > I also noticed the cpu binding refers to "riscv,cpu-intc" as well. > > That needs to be fixed too if there's a change. > > Only in the examples. I'd be fine with dropping them, but let's keep > that separate from the interrupt support. You need to sort out how this is all tied together and works because right now you are supporting 2 ways and one is undocumented and the other is invalid. Changing things later is only going to be more painful. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 08, 2018 at 10:15:58AM -0600, Rob Herring wrote: > 'interrupts' (via > interrupt-parent) or 'interrupts-extended' has to point to an > 'interrupt-controller' node. I guess you could make the cpu nodes > interrupt-controllers. That's a bit strange, but I can't think of a > reason why that wouldn't work. It could work, and would actually match how the hardware works fairly closely. > Just because the cpu-intc is not made to be an irqchip in the kernel > doesn't mean it can't still be represented as an interrupt-controller > in DT. It shouldn't be designed just around how some OS happens to > implement things. Independent of how you implement it, there isn't really such a thing as the cpu-intc. The CPU itself has a number of exceptions, that are all handled the same way. One of them just happens to be the connection to an external interrupt controller. That being said I'm fine with keeping up the pretence (at least in DT) that it is a separate entity and resubmit the cpu-intc docs given how widespread they exist already. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 08 Aug 2018 09:15:58 PDT (-0700), robh+dt@kernel.org wrote: > On Wed, Aug 8, 2018 at 8:59 AM Christoph Hellwig <hch@lst.de> wrote: >> >> On Wed, Aug 08, 2018 at 08:29:50AM -0600, Rob Herring wrote: >> > Version numbers on the individual patches would be nice... >> >> We've never done these in the subsystems I'm involved with. Too >> much clutter in the subject lines for information that is easily >> deductable. > > Unfortunately not in Gmail which doesn't thread properly. But > patchwork also provides the version tag which I use to sort my > reviews. > >> > > +Example: >> > > + >> > > + plic: interrupt-controller@c000000 { >> > > + #address-cells = <0>; >> > > + #interrupt-cells = <1>; >> > > + compatible = "riscv,plic0"; >> > > + interrupt-controller; >> > > + interrupts-extended = < >> > > + &cpu0-intc 11 >> > > + &cpu1-intc 11 &cpu1-intc 9 >> > > + &cpu2-intc 11 &cpu2-intc 9 >> > > + &cpu3-intc 11 &cpu3-intc 9 >> > > + &cpu4-intc 11 &cpu4-intc 9>; >> > >> > I'm confused why this is still here if you are dropping the cpu intc binding? >> >> We need some parent that identifies the core (hart in RISC-V terminology). >> The way the code now works is that it just walks up the parent chain >> until it finds a CPU node, so it either accepts the legacy intc node >> inbetween, or it accepts the cpu node directly as the intc node is pointless. >> >> I guess for the documentation we should instead just point to the >> "riscv" cpu nodes instead? > > That's not valid and dtc will tell you that. 'interrupts' (via > interrupt-parent) or 'interrupts-extended' has to point to an > 'interrupt-controller' node. I guess you could make the cpu nodes > interrupt-controllers. That's a bit strange, but I can't think of a > reason why that wouldn't work. > > Just because the cpu-intc is not made to be an irqchip in the kernel > doesn't mean it can't still be represented as an interrupt-controller > in DT. It shouldn't be designed just around how some OS happens to > implement things. FWIW, I like this approach. There is an interrupt widget in the hardware, so having the device tree represent it seems like a good idea. >> > I also noticed the cpu binding refers to "riscv,cpu-intc" as well. >> > That needs to be fixed too if there's a change. >> >> Only in the examples. I'd be fine with dropping them, but let's keep >> that separate from the interrupt support. > > You need to sort out how this is all tied together and works because > right now you are supporting 2 ways and one is undocumented and the > other is invalid. Changing things later is only going to be more > painful. > > Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt new file mode 100644 index 000000000000..bbfa61cf8d3f --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt @@ -0,0 +1,57 @@ +SiFive Platform-Level Interrupt Controller (PLIC) +------------------------------------------------- + +SiFive SOCs include an implementation of the Platform-Level Interrupt Controller +(PLIC) high-level specification in the RISC-V Privileged Architecture +specification. The PLIC connects all external interrupts in the system to all +hart contexts in the system, via the external interrupt source in each hart. + +A hart context is a privilege mode in a hardware execution thread. For example, +in an 4 core system with 2-way SMT, you have 8 harts and probably at least two +privilege modes per hart; machine mode and supervisor mode. + +Each interrupt can be enabled on per-context basis. Any context can claim +a pending enabled interrupt and then release it once it has been handled. + +Each interrupt has a configurable priority. Higher priority interrupts are +serviced first. Each context can specify a priority threshold. Interrupts +with priority below this threshold will not cause the PLIC to raise its +interrupt line leading to the context. + +While the PLIC supports both edge-triggered and level-triggered interrupts, +interrupt handlers are oblivious to this distinction and therefore it is not +specified in the PLIC device-tree binding. + +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the +"sifive,plic0" device is a concrete implementation of the PLIC that contains a +specific memory layout, which is documented in chapter 8 of the SiFive U5 +Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>. + +Required properties: +- compatible : "sifive,plic0". +- #address-cells : should be <0>. +- #interrupt-cells : should be <1>. +- interrupt-controller : Identifies the node as an interrupt controller. +- reg : Should contain 1 register range (address and length). +- interrupts-extended : Specifies which contexts are connected to the PLIC, + with "-1" specifying that a context is not present. The nodes pointed + to should be "riscv" HART nodes, or eventually be parented by such nodes. +- riscv,ndev: Specifies how many external interrupts are supported by + this controller. + +Example: + + plic: interrupt-controller@c000000 { + #address-cells = <0>; + #interrupt-cells = <1>; + compatible = "riscv,plic0"; + interrupt-controller; + interrupts-extended = < + &cpu0-intc 11 + &cpu1-intc 11 &cpu1-intc 9 + &cpu2-intc 11 &cpu2-intc 9 + &cpu3-intc 11 &cpu3-intc 9 + &cpu4-intc 11 &cpu4-intc 9>; + reg = <0xc000000 0x4000000>; + riscv,ndev = <10>; + };