| Message ID | 20220524172214.5104-2-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add PLIC support for Renesas RZ/Five SoC | expand |
On Tue, May 24, 2022 at 06:22:13PM +0100, Lad Prabhakar wrote: > Document Renesas RZ/Five (R9A07G043) SoC. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > .../sifive,plic-1.0.0.yaml | 38 +++++++++++++++++-- > 1 file changed, 35 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > index 27092c6a86c4..78ff31cb63e5 100644 > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > @@ -28,7 +28,10 @@ description: > > 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. > + specified in the PLIC device-tree binding for SiFive PLIC (and similar PLIC's), > + but for the Renesas RZ/Five Soc (AX45MP AndesCore) which has NCEPLIC100 we need > + to specify the interrupt type as the flow for EDGE interrupts is different > + compared to LEVEL interrupts. > > While the RISC-V ISA doesn't specify a memory layout for the PLIC, the > "sifive,plic-1.0.0" device is a concrete implementation of the PLIC that > @@ -57,6 +60,7 @@ properties: > - enum: > - allwinner,sun20i-d1-plic > - const: thead,c900-plic > + - const: renesas-r9a07g043-plic > > reg: > maxItems: 1 > @@ -64,8 +68,7 @@ properties: > '#address-cells': > const: 0 > > - '#interrupt-cells': > - const: 1 > + '#interrupt-cells': true > > interrupt-controller: true > > @@ -91,6 +94,35 @@ required: > - interrupts-extended > - riscv,ndev > > +if: > + properties: > + compatible: > + contains: > + const: renesas-r9a07g043-plic > +then: > + properties: > + clocks: > + maxItems: 1 > + > + resets: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 Did you test this? The above properties won't be allowed because of additionalProperties below. You need to change it to 'unevaluatedProperties' or move these to the top level. > + > + '#interrupt-cells': > + const: 2 > + > + required: > + - clocks > + - resets > + - power-domains > + > +else: > + properties: > + '#interrupt-cells': > + const: 1 > + > additionalProperties: false > > examples: > -- > 2.25.1 > >
Hi Prabhakar, On Tue, May 24, 2022 at 7:22 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Document Renesas RZ/Five (R9A07G043) SoC. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Thanks for your patch! > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > @@ -28,7 +28,10 @@ description: > > 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. > + specified in the PLIC device-tree binding for SiFive PLIC (and similar PLIC's), > + but for the Renesas RZ/Five Soc (AX45MP AndesCore) which has NCEPLIC100 we need > + to specify the interrupt type as the flow for EDGE interrupts is different > + compared to LEVEL interrupts. > > While the RISC-V ISA doesn't specify a memory layout for the PLIC, the > "sifive,plic-1.0.0" device is a concrete implementation of the PLIC that > @@ -57,6 +60,7 @@ properties: > - enum: > - allwinner,sun20i-d1-plic > - const: thead,c900-plic > + - const: renesas-r9a07g043-plic renesas,r9a07g043-plic > > reg: > maxItems: 1 > @@ -64,8 +68,7 @@ properties: > '#address-cells': > const: 0 > > - '#interrupt-cells': > - const: 1 > + '#interrupt-cells': true > > interrupt-controller: true > > @@ -91,6 +94,35 @@ required: > - interrupts-extended > - riscv,ndev > > +if: > + properties: > + compatible: > + contains: > + const: renesas-r9a07g043-plic renesas,r9a07g043-plic > +then: > + properties: > + clocks: > + maxItems: 1 > + > + resets: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + > + '#interrupt-cells': > + const: 2 > + > + required: > + - clocks > + - resets > + - power-domains > + > +else: > + properties: > + '#interrupt-cells': > + const: 1 > + > additionalProperties: false > > examples: Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Rob, Thank you for the review. On Sun, Jun 5, 2022 at 3:23 PM Rob Herring <robh@kernel.org> wrote: > > On Tue, May 24, 2022 at 06:22:13PM +0100, Lad Prabhakar wrote: > > Document Renesas RZ/Five (R9A07G043) SoC. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > .../sifive,plic-1.0.0.yaml | 38 +++++++++++++++++-- > > 1 file changed, 35 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > index 27092c6a86c4..78ff31cb63e5 100644 > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > @@ -28,7 +28,10 @@ description: > > > > 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. > > + specified in the PLIC device-tree binding for SiFive PLIC (and similar PLIC's), > > + but for the Renesas RZ/Five Soc (AX45MP AndesCore) which has NCEPLIC100 we need > > + to specify the interrupt type as the flow for EDGE interrupts is different > > + compared to LEVEL interrupts. > > > > While the RISC-V ISA doesn't specify a memory layout for the PLIC, the > > "sifive,plic-1.0.0" device is a concrete implementation of the PLIC that > > @@ -57,6 +60,7 @@ properties: > > - enum: > > - allwinner,sun20i-d1-plic > > - const: thead,c900-plic > > + - const: renesas-r9a07g043-plic > > > > reg: > > maxItems: 1 > > @@ -64,8 +68,7 @@ properties: > > '#address-cells': > > const: 0 > > > > - '#interrupt-cells': > > - const: 1 > > + '#interrupt-cells': true > > > > interrupt-controller: true > > > > @@ -91,6 +94,35 @@ required: > > - interrupts-extended > > - riscv,ndev > > > > +if: > > + properties: > > + compatible: > > + contains: > > + const: renesas-r9a07g043-plic > > +then: > > + properties: > > + clocks: > > + maxItems: 1 > > + > > + resets: > > + maxItems: 1 > > + > > + power-domains: > > + maxItems: 1 > > Did you test this? The above properties won't be allowed because of > additionalProperties below. You need to change it to > 'unevaluatedProperties' or move these to the top level. > Yes I have run the dt_binding check. So with the below diff it does complain about the missing properties. prasmi@prasmi:~/work/renasas/renesas-drivers$ git diff Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml index 20ded037d444..bb14a4b1ec0a 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml @@ -130,7 +130,7 @@ examples: plic: interrupt-controller@c000000 { #address-cells = <0>; #interrupt-cells = <1>; - compatible = "sifive,fu540-c000-plic", "sifive,plic-1.0.0"; + compatible = "renesas-r9a07g043-plic"; interrupt-controller; interrupts-extended = <&cpu0_intc 11>, <&cpu1_intc 11>, <&cpu1_intc 9>, prasmi@prasmi:~/work/renasas/renesas-drivers$ make ARCH=riscv CROSS_COMPILE=riscv64-linux-gnu- dt_binding_check LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dts DTC Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb CHECK Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: interrupt-controller@c000000: #interrupt-cells:0:0: 2 was expected From schema: /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: interrupt-controller@c000000: 'clocks' is a required property From schema: /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: interrupt-controller@c000000: 'resets' is a required property From schema: /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: interrupt-controller@c000000: 'power-domains' is a required property From schema: /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml prasmi@prasmi:~/work/renasas/renesas-drivers$ prasmi@prasmi:~/work/renasas/renesas-drivers$ Is there something I'm missing here? Cheers, Prabhakar > > + > > + '#interrupt-cells': > > + const: 2 > > + > > + required: > > + - clocks > > + - resets > > + - power-domains > > + > > +else: > > + properties: > > + '#interrupt-cells': > > + const: 1 > > + > > additionalProperties: false > > > > examples: > > -- > > 2.25.1 > > > >
Hi Geert, Thank you for the review. On Thu, Jun 9, 2022 at 10:42 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Tue, May 24, 2022 at 7:22 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Document Renesas RZ/Five (R9A07G043) SoC. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Thanks for your patch! > > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > @@ -28,7 +28,10 @@ description: > > > > 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. > > + specified in the PLIC device-tree binding for SiFive PLIC (and similar PLIC's), > > + but for the Renesas RZ/Five Soc (AX45MP AndesCore) which has NCEPLIC100 we need > > + to specify the interrupt type as the flow for EDGE interrupts is different > > + compared to LEVEL interrupts. > > > > While the RISC-V ISA doesn't specify a memory layout for the PLIC, the > > "sifive,plic-1.0.0" device is a concrete implementation of the PLIC that > > @@ -57,6 +60,7 @@ properties: > > - enum: > > - allwinner,sun20i-d1-plic > > - const: thead,c900-plic > > + - const: renesas-r9a07g043-plic > > renesas,r9a07g043-plic > Agreed. > > > > reg: > > maxItems: 1 > > @@ -64,8 +68,7 @@ properties: > > '#address-cells': > > const: 0 > > > > - '#interrupt-cells': > > - const: 1 > > + '#interrupt-cells': true > > > > interrupt-controller: true > > > > @@ -91,6 +94,35 @@ required: > > - interrupts-extended > > - riscv,ndev > > > > +if: > > + properties: > > + compatible: > > + contains: > > + const: renesas-r9a07g043-plic > > renesas,r9a07g043-plic > ditto. Cheers, Prabhakar
On Fri, Jun 24, 2022 at 10:59:40AM +0100, Lad, Prabhakar wrote: > Hi Rob, > > Thank you for the review. > > On Sun, Jun 5, 2022 at 3:23 PM Rob Herring <robh@kernel.org> wrote: > > > > On Tue, May 24, 2022 at 06:22:13PM +0100, Lad Prabhakar wrote: > > > Document Renesas RZ/Five (R9A07G043) SoC. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > --- > > > .../sifive,plic-1.0.0.yaml | 38 +++++++++++++++++-- > > > 1 file changed, 35 insertions(+), 3 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > index 27092c6a86c4..78ff31cb63e5 100644 > > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > @@ -28,7 +28,10 @@ description: > > > > > > 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. > > > + specified in the PLIC device-tree binding for SiFive PLIC (and similar PLIC's), > > > + but for the Renesas RZ/Five Soc (AX45MP AndesCore) which has NCEPLIC100 we need > > > + to specify the interrupt type as the flow for EDGE interrupts is different > > > + compared to LEVEL interrupts. > > > > > > While the RISC-V ISA doesn't specify a memory layout for the PLIC, the > > > "sifive,plic-1.0.0" device is a concrete implementation of the PLIC that > > > @@ -57,6 +60,7 @@ properties: > > > - enum: > > > - allwinner,sun20i-d1-plic > > > - const: thead,c900-plic > > > + - const: renesas-r9a07g043-plic Also, this should be 'renesas,r9...' > > > > > > reg: > > > maxItems: 1 > > > @@ -64,8 +68,7 @@ properties: > > > '#address-cells': > > > const: 0 > > > > > > - '#interrupt-cells': > > > - const: 1 > > > + '#interrupt-cells': true > > > > > > interrupt-controller: true > > > > > > @@ -91,6 +94,35 @@ required: > > > - interrupts-extended > > > - riscv,ndev > > > > > > +if: > > > + properties: > > > + compatible: > > > + contains: > > > + const: renesas-r9a07g043-plic > > > +then: > > > + properties: > > > + clocks: > > > + maxItems: 1 > > > + > > > + resets: > > > + maxItems: 1 > > > + > > > + power-domains: > > > + maxItems: 1 > > > > Did you test this? The above properties won't be allowed because of > > additionalProperties below. You need to change it to > > 'unevaluatedProperties' or move these to the top level. > > > Yes I have run the dt_binding check. > > So with the below diff it does complain about the missing properties. > > prasmi@prasmi:~/work/renasas/renesas-drivers$ git diff > Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > index 20ded037d444..bb14a4b1ec0a 100644 > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > @@ -130,7 +130,7 @@ examples: > plic: interrupt-controller@c000000 { > #address-cells = <0>; > #interrupt-cells = <1>; > - compatible = "sifive,fu540-c000-plic", "sifive,plic-1.0.0"; > + compatible = "renesas-r9a07g043-plic"; > interrupt-controller; > interrupts-extended = <&cpu0_intc 11>, > <&cpu1_intc 11>, <&cpu1_intc 9>, > prasmi@prasmi:~/work/renasas/renesas-drivers$ make ARCH=riscv > CROSS_COMPILE=riscv64-linux-gnu- dt_binding_check > LINT Documentation/devicetree/bindings > CHKDT Documentation/devicetree/bindings/processed-schema.json > SCHEMA Documentation/devicetree/bindings/processed-schema.json > DTEX Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dts > DTC Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb > CHECK Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > interrupt-controller@c000000: #interrupt-cells:0:0: 2 was expected > From schema: > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > interrupt-controller@c000000: 'clocks' is a required property > From schema: > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > interrupt-controller@c000000: 'resets' is a required property > From schema: > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > interrupt-controller@c000000: 'power-domains' is a required property > From schema: > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > prasmi@prasmi:~/work/renasas/renesas-drivers$ > prasmi@prasmi:~/work/renasas/renesas-drivers$ > > Is there something I'm missing here? You've said these properties are required, but you didn't add them. If you don't have the above 3 properties, then it's not going to complain that they are present. But it will when you do add them for the reason I gave. Rob
On Wed, 06 Jul 2022 22:58:27 +0100, Rob Herring <robh@kernel.org> wrote: > > On Fri, Jun 24, 2022 at 10:59:40AM +0100, Lad, Prabhakar wrote: > > Hi Rob, > > > > Thank you for the review. > > > > On Sun, Jun 5, 2022 at 3:23 PM Rob Herring <robh@kernel.org> wrote: > > > > > > On Tue, May 24, 2022 at 06:22:13PM +0100, Lad Prabhakar wrote: > > > > Document Renesas RZ/Five (R9A07G043) SoC. > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > --- > > > > .../sifive,plic-1.0.0.yaml | 38 +++++++++++++++++-- > > > > 1 file changed, 35 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > > index 27092c6a86c4..78ff31cb63e5 100644 > > > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > > @@ -28,7 +28,10 @@ description: > > > > > > > > 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. > > > > + specified in the PLIC device-tree binding for SiFive PLIC (and similar PLIC's), > > > > + but for the Renesas RZ/Five Soc (AX45MP AndesCore) which has NCEPLIC100 we need > > > > + to specify the interrupt type as the flow for EDGE interrupts is different > > > > + compared to LEVEL interrupts. > > > > > > > > While the RISC-V ISA doesn't specify a memory layout for the PLIC, the > > > > "sifive,plic-1.0.0" device is a concrete implementation of the PLIC that > > > > @@ -57,6 +60,7 @@ properties: > > > > - enum: > > > > - allwinner,sun20i-d1-plic > > > > - const: thead,c900-plic > > > > + - const: renesas-r9a07g043-plic > > Also, this should be 'renesas,r9...' > > > > > > > > > reg: > > > > maxItems: 1 > > > > @@ -64,8 +68,7 @@ properties: > > > > '#address-cells': > > > > const: 0 > > > > > > > > - '#interrupt-cells': > > > > - const: 1 > > > > + '#interrupt-cells': true > > > > > > > > interrupt-controller: true > > > > > > > > @@ -91,6 +94,35 @@ required: > > > > - interrupts-extended > > > > - riscv,ndev > > > > > > > > +if: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + const: renesas-r9a07g043-plic > > > > +then: > > > > + properties: > > > > + clocks: > > > > + maxItems: 1 > > > > + > > > > + resets: > > > > + maxItems: 1 > > > > + > > > > + power-domains: > > > > + maxItems: 1 > > > > > > Did you test this? The above properties won't be allowed because of > > > additionalProperties below. You need to change it to > > > 'unevaluatedProperties' or move these to the top level. > > > > > Yes I have run the dt_binding check. > > > > So with the below diff it does complain about the missing properties. > > > > prasmi@prasmi:~/work/renasas/renesas-drivers$ git diff > > Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > index 20ded037d444..bb14a4b1ec0a 100644 > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > @@ -130,7 +130,7 @@ examples: > > plic: interrupt-controller@c000000 { > > #address-cells = <0>; > > #interrupt-cells = <1>; > > - compatible = "sifive,fu540-c000-plic", "sifive,plic-1.0.0"; > > + compatible = "renesas-r9a07g043-plic"; > > interrupt-controller; > > interrupts-extended = <&cpu0_intc 11>, > > <&cpu1_intc 11>, <&cpu1_intc 9>, > > prasmi@prasmi:~/work/renasas/renesas-drivers$ make ARCH=riscv > > CROSS_COMPILE=riscv64-linux-gnu- dt_binding_check > > LINT Documentation/devicetree/bindings > > CHKDT Documentation/devicetree/bindings/processed-schema.json > > SCHEMA Documentation/devicetree/bindings/processed-schema.json > > DTEX Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dts > > DTC Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb > > CHECK Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > > interrupt-controller@c000000: #interrupt-cells:0:0: 2 was expected > > From schema: > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > > interrupt-controller@c000000: 'clocks' is a required property > > From schema: > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > > interrupt-controller@c000000: 'resets' is a required property > > From schema: > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > > interrupt-controller@c000000: 'power-domains' is a required property > > From schema: > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > prasmi@prasmi:~/work/renasas/renesas-drivers$ > > prasmi@prasmi:~/work/renasas/renesas-drivers$ > > > > Is there something I'm missing here? > > You've said these properties are required, but you didn't add them. > > If you don't have the above 3 properties, then it's not going to > complain that they are present. But it will when you do add them for the > reason I gave. Can you please have a look at the latest instance[1][2] of this series, as posted by Samuel? I've provisionally queued it, but only on the provision that you would eventually ack these patches. Thanks, M. [1] https://lore.kernel.org/r/20220630100241.35233-2-samuel@sholland.org [2] https://lore.kernel.org/r/20220630100241.35233-4-samuel@sholland.org
On Thu, Jul 07, 2022 at 10:51:33AM +0100, Marc Zyngier wrote: > On Wed, 06 Jul 2022 22:58:27 +0100, > Rob Herring <robh@kernel.org> wrote: > > > > On Fri, Jun 24, 2022 at 10:59:40AM +0100, Lad, Prabhakar wrote: > > > Hi Rob, > > > > > > Thank you for the review. > > > > > > On Sun, Jun 5, 2022 at 3:23 PM Rob Herring <robh@kernel.org> wrote: > > > > > > > > On Tue, May 24, 2022 at 06:22:13PM +0100, Lad Prabhakar wrote: > > > > > Document Renesas RZ/Five (R9A07G043) SoC. > > > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > --- > > > > > .../sifive,plic-1.0.0.yaml | 38 +++++++++++++++++-- > > > > > 1 file changed, 35 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > > > index 27092c6a86c4..78ff31cb63e5 100644 > > > > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > > > @@ -28,7 +28,10 @@ description: > > > > > > > > > > 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. > > > > > + specified in the PLIC device-tree binding for SiFive PLIC (and similar PLIC's), > > > > > + but for the Renesas RZ/Five Soc (AX45MP AndesCore) which has NCEPLIC100 we need > > > > > + to specify the interrupt type as the flow for EDGE interrupts is different > > > > > + compared to LEVEL interrupts. > > > > > > > > > > While the RISC-V ISA doesn't specify a memory layout for the PLIC, the > > > > > "sifive,plic-1.0.0" device is a concrete implementation of the PLIC that > > > > > @@ -57,6 +60,7 @@ properties: > > > > > - enum: > > > > > - allwinner,sun20i-d1-plic > > > > > - const: thead,c900-plic > > > > > + - const: renesas-r9a07g043-plic > > > > Also, this should be 'renesas,r9...' > > > > > > > > > > > > reg: > > > > > maxItems: 1 > > > > > @@ -64,8 +68,7 @@ properties: > > > > > '#address-cells': > > > > > const: 0 > > > > > > > > > > - '#interrupt-cells': > > > > > - const: 1 > > > > > + '#interrupt-cells': true > > > > > > > > > > interrupt-controller: true > > > > > > > > > > @@ -91,6 +94,35 @@ required: > > > > > - interrupts-extended > > > > > - riscv,ndev > > > > > > > > > > +if: > > > > > + properties: > > > > > + compatible: > > > > > + contains: > > > > > + const: renesas-r9a07g043-plic > > > > > +then: > > > > > + properties: > > > > > + clocks: > > > > > + maxItems: 1 > > > > > + > > > > > + resets: > > > > > + maxItems: 1 > > > > > + > > > > > + power-domains: > > > > > + maxItems: 1 > > > > > > > > Did you test this? The above properties won't be allowed because of > > > > additionalProperties below. You need to change it to > > > > 'unevaluatedProperties' or move these to the top level. > > > > > > > Yes I have run the dt_binding check. > > > > > > So with the below diff it does complain about the missing properties. > > > > > > prasmi@prasmi:~/work/renasas/renesas-drivers$ git diff > > > Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > index 20ded037d444..bb14a4b1ec0a 100644 > > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > @@ -130,7 +130,7 @@ examples: > > > plic: interrupt-controller@c000000 { > > > #address-cells = <0>; > > > #interrupt-cells = <1>; > > > - compatible = "sifive,fu540-c000-plic", "sifive,plic-1.0.0"; > > > + compatible = "renesas-r9a07g043-plic"; > > > interrupt-controller; > > > interrupts-extended = <&cpu0_intc 11>, > > > <&cpu1_intc 11>, <&cpu1_intc 9>, > > > prasmi@prasmi:~/work/renasas/renesas-drivers$ make ARCH=riscv > > > CROSS_COMPILE=riscv64-linux-gnu- dt_binding_check > > > LINT Documentation/devicetree/bindings > > > CHKDT Documentation/devicetree/bindings/processed-schema.json > > > SCHEMA Documentation/devicetree/bindings/processed-schema.json > > > DTEX Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dts > > > DTC Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb > > > CHECK Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb > > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > > > interrupt-controller@c000000: #interrupt-cells:0:0: 2 was expected > > > From schema: > > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > > > interrupt-controller@c000000: 'clocks' is a required property > > > From schema: > > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > > > interrupt-controller@c000000: 'resets' is a required property > > > From schema: > > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.example.dtb: > > > interrupt-controller@c000000: 'power-domains' is a required property > > > From schema: > > > /home/prasmi/work/renasas/renesas-drivers/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > > prasmi@prasmi:~/work/renasas/renesas-drivers$ > > > prasmi@prasmi:~/work/renasas/renesas-drivers$ > > > > > > Is there something I'm missing here? > > > > You've said these properties are required, but you didn't add them. > > > > If you don't have the above 3 properties, then it's not going to > > complain that they are present. But it will when you do add them for the > > reason I gave. > > Can you please have a look at the latest instance[1][2] of this > series, as posted by Samuel? I've provisionally queued it, but only on > the provision that you would eventually ack these patches. I did already[1]. They passed checks, were already in linux-next, and I didn't see anything major needing comments, so I marked it N/A (meaning someone else applies it) without comment. Rob [1] https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20220630100241.35233-2-samuel@sholland.org/
On Tue, 12 Jul 2022 19:19:16 +0100, Rob Herring <robh@kernel.org> wrote: > > On Thu, Jul 07, 2022 at 10:51:33AM +0100, Marc Zyngier wrote: > > > Can you please have a look at the latest instance[1][2] of this > > series, as posted by Samuel? I've provisionally queued it, but only on > > the provision that you would eventually ack these patches. > > I did already[1]. They passed checks, were already in linux-next, and I > didn't see anything major needing comments, so I marked it N/A (meaning > someone else applies it) without comment. > > Rob > > [1] https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20220630100241.35233-2-samuel@sholland.org/ How are people supposed to track this if it doesn't appear on the ML? That's not really an ack, AFAICT. That's a "I don't care". Does it mean I'm free to take any random DT patch unless you or a bot shouts? I'd rather know. M. (puzzled)
On Tue, Jul 12, 2022 at 08:21:27PM +0100, Marc Zyngier wrote: > On Tue, 12 Jul 2022 19:19:16 +0100, > Rob Herring <robh@kernel.org> wrote: > > > > On Thu, Jul 07, 2022 at 10:51:33AM +0100, Marc Zyngier wrote: > > > > > Can you please have a look at the latest instance[1][2] of this > > > series, as posted by Samuel? I've provisionally queued it, but only on > > > the provision that you would eventually ack these patches. > > > > I did already[1]. They passed checks, were already in linux-next, and I > > didn't see anything major needing comments, so I marked it N/A (meaning > > someone else applies it) without comment. > > > > Rob > > > > [1] https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20220630100241.35233-2-samuel@sholland.org/ > > How are people supposed to track this if it doesn't appear on the ML? Look at patchwork? How am I supposed to track maintainers that will rebase what they have in next to add acks and those that won't? > That's not really an ack, AFAICT. That's a "I don't care". I still look at it, so really it's an implicit ack. In this case, I probably just saw the irqchip-bot mail and missed your reply to the cover. > Does it mean I'm free to take any random DT patch unless you or a bot > shouts? I'd rather know. Wait for acked/reviewed-by to apply? Isn't that the process? Rob
diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml index 27092c6a86c4..78ff31cb63e5 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml @@ -28,7 +28,10 @@ description: 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. + specified in the PLIC device-tree binding for SiFive PLIC (and similar PLIC's), + but for the Renesas RZ/Five Soc (AX45MP AndesCore) which has NCEPLIC100 we need + to specify the interrupt type as the flow for EDGE interrupts is different + compared to LEVEL interrupts. While the RISC-V ISA doesn't specify a memory layout for the PLIC, the "sifive,plic-1.0.0" device is a concrete implementation of the PLIC that @@ -57,6 +60,7 @@ properties: - enum: - allwinner,sun20i-d1-plic - const: thead,c900-plic + - const: renesas-r9a07g043-plic reg: maxItems: 1 @@ -64,8 +68,7 @@ properties: '#address-cells': const: 0 - '#interrupt-cells': - const: 1 + '#interrupt-cells': true interrupt-controller: true @@ -91,6 +94,35 @@ required: - interrupts-extended - riscv,ndev +if: + properties: + compatible: + contains: + const: renesas-r9a07g043-plic +then: + properties: + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + power-domains: + maxItems: 1 + + '#interrupt-cells': + const: 2 + + required: + - clocks + - resets + - power-domains + +else: + properties: + '#interrupt-cells': + const: 1 + additionalProperties: false examples:
Document Renesas RZ/Five (R9A07G043) SoC. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- .../sifive,plic-1.0.0.yaml | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-)