| Message ID | 20210830041729.237252-7-anup.patel@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Linux RISC-V ACLINT Support | expand |
On Mon, Aug 30, 2021 at 09:47:24AM +0530, Anup Patel wrote: > The Linux RISC-V now treats IPIs as regular per-CPU IRQs. This means > we have to create a IPI interrupt domain to use CLINT IPI functionality > hence requiring a "interrupt-controller" and "#interrupt-cells" DT > property in CLINT DT nodes. > > Impact of this CLINT DT bindings change only affects Linux RISC-V > NoMMU kernel and has no effect of existing M-mode runtime firmwares > (i.e. OpenSBI). It appears to me you should fix Linux to not need these 2 useless properties. I say useless because #interrupt-cells being 0 is pretty useless. > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > --- > .../bindings/timer/sifive,clint.yaml | 20 ++++++++++++++----- > arch/riscv/boot/dts/canaan/k210.dtsi | 2 ++ > .../boot/dts/microchip/microchip-mpfs.dtsi | 2 ++ > 3 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > index a35952f48742..9c8ef9f4094f 100644 > --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml > +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > @@ -43,6 +43,12 @@ properties: > > interrupts-extended: > minItems: 1 > + maxItems: 4095 > + > + "#interrupt-cells": > + const: 0 > + > + interrupt-controller: true > > additionalProperties: false > > @@ -50,15 +56,19 @@ required: > - compatible > - reg > - interrupts-extended > + - interrupt-controller > + - "#interrupt-cells" > > examples: > - | > timer@2000000 { > compatible = "sifive,fu540-c000-clint", "sifive,clint0"; > - interrupts-extended = <&cpu1intc 3 &cpu1intc 7 > - &cpu2intc 3 &cpu2intc 7 > - &cpu3intc 3 &cpu3intc 7 > - &cpu4intc 3 &cpu4intc 7>; > - reg = <0x2000000 0x10000>; > + interrupts-extended = <&cpu1intc 3>, <&cpu1intc 7>, > + <&cpu2intc 3>, <&cpu2intc 7>, > + <&cpu3intc 3>, <&cpu3intc 7>, > + <&cpu4intc 3>, <&cpu4intc 7>; > + reg = <0x2000000 0x10000>; > + interrupt-controller; > + #interrupt-cells = <0>; > }; > ... > diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi b/arch/riscv/boot/dts/canaan/k210.dtsi > index 5e8ca8142482..67dcda1efadb 100644 > --- a/arch/riscv/boot/dts/canaan/k210.dtsi > +++ b/arch/riscv/boot/dts/canaan/k210.dtsi > @@ -105,6 +105,8 @@ clint0: timer@2000000 { > reg = <0x2000000 0xC000>; > interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 > &cpu1_intc 3 &cpu1_intc 7>; > + #interrupt-cells = <0>; > + interrupt-controller; > }; > > plic0: interrupt-controller@c000000 { > diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi > index b9819570a7d1..67fb41439f20 100644 > --- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi > +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi > @@ -168,6 +168,8 @@ &cpu1_intc 3 &cpu1_intc 7 > &cpu2_intc 3 &cpu2_intc 7 > &cpu3_intc 3 &cpu3_intc 7 > &cpu4_intc 3 &cpu4_intc 7>; > + #interrupt-cells = <0>; > + interrupt-controller; > }; > > plic: interrupt-controller@c000000 { > -- > 2.25.1 > >
On Wed, Sep 1, 2021 at 6:59 AM Rob Herring <robh@kernel.org> wrote: > > On Mon, Aug 30, 2021 at 09:47:24AM +0530, Anup Patel wrote: > > The Linux RISC-V now treats IPIs as regular per-CPU IRQs. This means > > we have to create a IPI interrupt domain to use CLINT IPI functionality > > hence requiring a "interrupt-controller" and "#interrupt-cells" DT > > property in CLINT DT nodes. > > > > Impact of this CLINT DT bindings change only affects Linux RISC-V > > NoMMU kernel and has no effect of existing M-mode runtime firmwares > > (i.e. OpenSBI). > > It appears to me you should fix Linux to not need these 2 useless > properties. I say useless because #interrupt-cells being 0 is pretty > useless. Linux IRQCHIP framework only probes IRQCHIP DT nodes which have "interrupt-controller" DT property. The "interrupt-cells" DT property can be removed because as an interrupt controller SiFive CLINT will only provide IPIs to arch code. Regards, Anup > > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > --- > > .../bindings/timer/sifive,clint.yaml | 20 ++++++++++++++----- > > arch/riscv/boot/dts/canaan/k210.dtsi | 2 ++ > > .../boot/dts/microchip/microchip-mpfs.dtsi | 2 ++ > > 3 files changed, 19 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > > index a35952f48742..9c8ef9f4094f 100644 > > --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml > > +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > > @@ -43,6 +43,12 @@ properties: > > > > interrupts-extended: > > minItems: 1 > > + maxItems: 4095 > > + > > + "#interrupt-cells": > > + const: 0 > > + > > + interrupt-controller: true > > > > additionalProperties: false > > > > @@ -50,15 +56,19 @@ required: > > - compatible > > - reg > > - interrupts-extended > > + - interrupt-controller > > + - "#interrupt-cells" > > > > examples: > > - | > > timer@2000000 { > > compatible = "sifive,fu540-c000-clint", "sifive,clint0"; > > - interrupts-extended = <&cpu1intc 3 &cpu1intc 7 > > - &cpu2intc 3 &cpu2intc 7 > > - &cpu3intc 3 &cpu3intc 7 > > - &cpu4intc 3 &cpu4intc 7>; > > - reg = <0x2000000 0x10000>; > > + interrupts-extended = <&cpu1intc 3>, <&cpu1intc 7>, > > + <&cpu2intc 3>, <&cpu2intc 7>, > > + <&cpu3intc 3>, <&cpu3intc 7>, > > + <&cpu4intc 3>, <&cpu4intc 7>; > > + reg = <0x2000000 0x10000>; > > + interrupt-controller; > > + #interrupt-cells = <0>; > > }; > > ... > > diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi b/arch/riscv/boot/dts/canaan/k210.dtsi > > index 5e8ca8142482..67dcda1efadb 100644 > > --- a/arch/riscv/boot/dts/canaan/k210.dtsi > > +++ b/arch/riscv/boot/dts/canaan/k210.dtsi > > @@ -105,6 +105,8 @@ clint0: timer@2000000 { > > reg = <0x2000000 0xC000>; > > interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 > > &cpu1_intc 3 &cpu1_intc 7>; > > + #interrupt-cells = <0>; > > + interrupt-controller; > > }; > > > > plic0: interrupt-controller@c000000 { > > diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi > > index b9819570a7d1..67fb41439f20 100644 > > --- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi > > +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi > > @@ -168,6 +168,8 @@ &cpu1_intc 3 &cpu1_intc 7 > > &cpu2_intc 3 &cpu2_intc 7 > > &cpu3_intc 3 &cpu3_intc 7 > > &cpu4_intc 3 &cpu4_intc 7>; > > + #interrupt-cells = <0>; > > + interrupt-controller; > > }; > > > > plic: interrupt-controller@c000000 { > > -- > > 2.25.1 > > > >
On Wed, Sep 1, 2021 at 7:00 AM Anup Patel <anup@brainfault.org> wrote: > > On Wed, Sep 1, 2021 at 6:59 AM Rob Herring <robh@kernel.org> wrote: > > > > On Mon, Aug 30, 2021 at 09:47:24AM +0530, Anup Patel wrote: > > > The Linux RISC-V now treats IPIs as regular per-CPU IRQs. This means > > > we have to create a IPI interrupt domain to use CLINT IPI functionality > > > hence requiring a "interrupt-controller" and "#interrupt-cells" DT > > > property in CLINT DT nodes. > > > > > > Impact of this CLINT DT bindings change only affects Linux RISC-V > > > NoMMU kernel and has no effect of existing M-mode runtime firmwares > > > (i.e. OpenSBI). > > > > It appears to me you should fix Linux to not need these 2 useless > > properties. I say useless because #interrupt-cells being 0 is pretty > > useless. > > Linux IRQCHIP framework only probes IRQCHIP DT nodes which > have "interrupt-controller" DT property. Right, I believe I wrote that... So what would it look like to fix that? The simplest thing is just drop the check for 'interrupt-controller'. That's just a sanity check and we have other ways to do that now (schemas). Do you need this early? You can always implement your own initcall. > The "interrupt-cells" DT property > can be removed because as an interrupt controller SiFive CLINT > will only provide IPIs to arch code. The schema will disagree. Rob
On Thu, Sep 2, 2021 at 5:48 AM Rob Herring <robh@kernel.org> wrote: > > On Wed, Sep 1, 2021 at 7:00 AM Anup Patel <anup@brainfault.org> wrote: > > > > On Wed, Sep 1, 2021 at 6:59 AM Rob Herring <robh@kernel.org> wrote: > > > > > > On Mon, Aug 30, 2021 at 09:47:24AM +0530, Anup Patel wrote: > > > > The Linux RISC-V now treats IPIs as regular per-CPU IRQs. This means > > > > we have to create a IPI interrupt domain to use CLINT IPI functionality > > > > hence requiring a "interrupt-controller" and "#interrupt-cells" DT > > > > property in CLINT DT nodes. > > > > > > > > Impact of this CLINT DT bindings change only affects Linux RISC-V > > > > NoMMU kernel and has no effect of existing M-mode runtime firmwares > > > > (i.e. OpenSBI). > > > > > > It appears to me you should fix Linux to not need these 2 useless > > > properties. I say useless because #interrupt-cells being 0 is pretty > > > useless. > > > > Linux IRQCHIP framework only probes IRQCHIP DT nodes which > > have "interrupt-controller" DT property. > > Right, I believe I wrote that... So what would it look like to fix > that? The simplest thing is just drop the check for > 'interrupt-controller'. That's just a sanity check and we have other > ways to do that now (schemas). Do you need this early? You can always > implement your own initcall. Okay, let me first try to fix this in the driver itself. Most likely, we will not require changes in this DT binding. > > > > The "interrupt-cells" DT property > > can be removed because as an interrupt controller SiFive CLINT > > will only provide IPIs to arch code. > > The schema will disagree. Okay. > > Rob Regards, Anup
diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml index a35952f48742..9c8ef9f4094f 100644 --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml @@ -43,6 +43,12 @@ properties: interrupts-extended: minItems: 1 + maxItems: 4095 + + "#interrupt-cells": + const: 0 + + interrupt-controller: true additionalProperties: false @@ -50,15 +56,19 @@ required: - compatible - reg - interrupts-extended + - interrupt-controller + - "#interrupt-cells" examples: - | timer@2000000 { compatible = "sifive,fu540-c000-clint", "sifive,clint0"; - interrupts-extended = <&cpu1intc 3 &cpu1intc 7 - &cpu2intc 3 &cpu2intc 7 - &cpu3intc 3 &cpu3intc 7 - &cpu4intc 3 &cpu4intc 7>; - reg = <0x2000000 0x10000>; + interrupts-extended = <&cpu1intc 3>, <&cpu1intc 7>, + <&cpu2intc 3>, <&cpu2intc 7>, + <&cpu3intc 3>, <&cpu3intc 7>, + <&cpu4intc 3>, <&cpu4intc 7>; + reg = <0x2000000 0x10000>; + interrupt-controller; + #interrupt-cells = <0>; }; ... diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi b/arch/riscv/boot/dts/canaan/k210.dtsi index 5e8ca8142482..67dcda1efadb 100644 --- a/arch/riscv/boot/dts/canaan/k210.dtsi +++ b/arch/riscv/boot/dts/canaan/k210.dtsi @@ -105,6 +105,8 @@ clint0: timer@2000000 { reg = <0x2000000 0xC000>; interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 &cpu1_intc 3 &cpu1_intc 7>; + #interrupt-cells = <0>; + interrupt-controller; }; plic0: interrupt-controller@c000000 { diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi index b9819570a7d1..67fb41439f20 100644 --- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi @@ -168,6 +168,8 @@ &cpu1_intc 3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3 &cpu3_intc 7 &cpu4_intc 3 &cpu4_intc 7>; + #interrupt-cells = <0>; + interrupt-controller; }; plic: interrupt-controller@c000000 {
The Linux RISC-V now treats IPIs as regular per-CPU IRQs. This means we have to create a IPI interrupt domain to use CLINT IPI functionality hence requiring a "interrupt-controller" and "#interrupt-cells" DT property in CLINT DT nodes. Impact of this CLINT DT bindings change only affects Linux RISC-V NoMMU kernel and has no effect of existing M-mode runtime firmwares (i.e. OpenSBI). Signed-off-by: Anup Patel <anup.patel@wdc.com> --- .../bindings/timer/sifive,clint.yaml | 20 ++++++++++++++----- arch/riscv/boot/dts/canaan/k210.dtsi | 2 ++ .../boot/dts/microchip/microchip-mpfs.dtsi | 2 ++ 3 files changed, 19 insertions(+), 5 deletions(-)