Messages in this thread | | | Date | Wed, 08 Aug 2018 13:49:37 -0700 (PDT) | Subject | Re: [PATCH 6/8] dt-bindings: interrupt-controller: RISC-V PLIC documentation | From | Palmer Dabbelt <> |
| |
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
| |