2022-07-21 22:16:33

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: soc: renesas: renesas,rzg2l-sysc: Document RZ/Five SoC

Document RZ/Five (R9A07G043) SYSC bindings. SYSC block found on the
RZ/Five SoC is almost identical to one found on the RZ/G2L (and alike)
SoC's. To differentiate RZ/G2UL from RZ/Five, "-rzfive" is included in
the compatible string for the RZ/Five SoC as there are no interrupts
from the SYSC block to the core.

Signed-off-by: Lad Prabhakar <[email protected]>
---
.../soc/renesas/renesas,rzg2l-sysc.yaml | 56 +++++++++++++------
1 file changed, 39 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
index ce2875c89329..bdaf05f8b29b 100644
--- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
+++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
@@ -20,35 +20,57 @@ description:
properties:
compatible:
enum:
- - renesas,r9a07g043-sysc # RZ/G2UL
- - renesas,r9a07g044-sysc # RZ/G2{L,LC}
- - renesas,r9a07g054-sysc # RZ/V2L
+ - renesas,r9a07g043-rzfive-sysc # RZ/Five
+ - renesas,r9a07g043-sysc # RZ/G2UL
+ - renesas,r9a07g044-sysc # RZ/G2{L,LC}
+ - renesas,r9a07g054-sysc # RZ/V2L

reg:
maxItems: 1

- interrupts:
- items:
- - description: CA55/CM33 Sleep/Software Standby Mode request interrupt
- - description: CA55 Software Standby Mode release request interrupt
- - description: CM33 Software Standby Mode release request interrupt
- - description: CA55 ACE Asynchronous Bridge Master/Slave interface deny request interrupt
+ interrupts: true

- interrupt-names:
- items:
- - const: lpm_int
- - const: ca55stbydone_int
- - const: cm33stbyr_int
- - const: ca55_deny
+ interrupt-names: true

required:
- compatible
- reg
- - interrupts
- - interrupt-names

additionalProperties: false

+allOf:
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - renesas,r9a07g043-rzfive-sysc
+ then:
+ properties:
+ interrupts:
+ items:
+ - description: CA55/CM33 Sleep/Software Standby Mode request interrupt
+ - description: CA55 Software Standby Mode release request interrupt
+ - description: CM33 Software Standby Mode release request interrupt
+ - description: CA55 ACE Asynchronous Bridge Master/Slave interface deny request interrupt
+
+ interrupt-names:
+ items:
+ - const: lpm_int
+ - const: ca55stbydone_int
+ - const: cm33stbyr_int
+ - const: ca55_deny
+
+ required:
+ - interrupts
+ - interrupt-names
+
+ else:
+ properties:
+ interrupts: false
+ interrupt-names: false
+
examples:
- |
#include <dt-bindings/interrupt-controller/arm-gic.h>
--
2.25.1


2022-07-22 09:54:51

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: soc: renesas: renesas,rzg2l-sysc: Document RZ/Five SoC

Hi Prabhakar,

On Fri, Jul 22, 2022 at 12:15 AM Lad Prabhakar
<[email protected]> wrote:
> Document RZ/Five (R9A07G043) SYSC bindings. SYSC block found on the
> RZ/Five SoC is almost identical to one found on the RZ/G2L (and alike)
> SoC's. To differentiate RZ/G2UL from RZ/Five, "-rzfive" is included in
> the compatible string for the RZ/Five SoC as there are no interrupts
> from the SYSC block to the core.
>
> Signed-off-by: Lad Prabhakar <[email protected]>

Thanks for your patch!

> ---
> .../soc/renesas/renesas,rzg2l-sysc.yaml | 56 +++++++++++++------
> 1 file changed, 39 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> index ce2875c89329..bdaf05f8b29b 100644
> --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> @@ -20,35 +20,57 @@ description:
> properties:
> compatible:
> enum:
> - - renesas,r9a07g043-sysc # RZ/G2UL
> - - renesas,r9a07g044-sysc # RZ/G2{L,LC}
> - - renesas,r9a07g054-sysc # RZ/V2L
> + - renesas,r9a07g043-rzfive-sysc # RZ/Five

renesas,r9a07g043f-sysc?

But I'm wondering if we really need a different compatible value?
It looks like both blocks differ only in external wiring, so if
anything needs to be handled differently (the removed/added registers
are related to CPU topology), that can be inferred from the system
topology (or even #ifdef CONFIG_{ARM64,RISCV} ;-)

> + - renesas,r9a07g043-sysc # RZ/G2UL
> + - renesas,r9a07g044-sysc # RZ/G2{L,LC}
> + - renesas,r9a07g054-sysc # RZ/V2L
>
> reg:
> maxItems: 1
>
> - interrupts:
> - items:
> - - description: CA55/CM33 Sleep/Software Standby Mode request interrupt
> - - description: CA55 Software Standby Mode release request interrupt
> - - description: CM33 Software Standby Mode release request interrupt
> - - description: CA55 ACE Asynchronous Bridge Master/Slave interface deny request interrupt
> + interrupts: true
>
> - interrupt-names:
> - items:
> - - const: lpm_int
> - - const: ca55stbydone_int
> - - const: cm33stbyr_int
> - - const: ca55_deny
> + interrupt-names: true
>
> required:
> - compatible
> - reg
> - - interrupts
> - - interrupt-names
>
> additionalProperties: false
>
> +allOf:
> + - if:
> + not:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - renesas,r9a07g043-rzfive-sysc
> + then:
> + properties:
> + interrupts:
> + items:
> + - description: CA55/CM33 Sleep/Software Standby Mode request interrupt
> + - description: CA55 Software Standby Mode release request interrupt
> + - description: CM33 Software Standby Mode release request interrupt
> + - description: CA55 ACE Asynchronous Bridge Master/Slave interface deny request interrupt
> +
> + interrupt-names:
> + items:
> + - const: lpm_int
> + - const: ca55stbydone_int
> + - const: cm33stbyr_int
> + - const: ca55_deny
> +
> + required:
> + - interrupts
> + - interrupt-names
> +
> + else:
> + properties:
> + interrupts: false
> + interrupt-names: false

Do all interrupts{,-names} have to be moved?
Wouldn't it be sufficient to just have

if [...]
then:
required:
- interrupts
- interrupt-names
else:
properties:
interrupts: false
interrupt-names: false

?

But again, without a new compatible value, you could just make
interrupts{,-names} not required?

> +
> examples:
> - |
> #include <dt-bindings/interrupt-controller/arm-gic.h>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-07-22 10:24:12

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: soc: renesas: renesas,rzg2l-sysc: Document RZ/Five SoC

Hi Geert,

Thank you for the review.

On Fri, Jul 22, 2022 at 10:31 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Fri, Jul 22, 2022 at 12:15 AM Lad Prabhakar
> <[email protected]> wrote:
> > Document RZ/Five (R9A07G043) SYSC bindings. SYSC block found on the
> > RZ/Five SoC is almost identical to one found on the RZ/G2L (and alike)
> > SoC's. To differentiate RZ/G2UL from RZ/Five, "-rzfive" is included in
> > the compatible string for the RZ/Five SoC as there are no interrupts
> > from the SYSC block to the core.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
>
> Thanks for your patch!
>
> > ---
> > .../soc/renesas/renesas,rzg2l-sysc.yaml | 56 +++++++++++++------
> > 1 file changed, 39 insertions(+), 17 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> > index ce2875c89329..bdaf05f8b29b 100644
> > --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> > +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> > @@ -20,35 +20,57 @@ description:
> > properties:
> > compatible:
> > enum:
> > - - renesas,r9a07g043-sysc # RZ/G2UL
> > - - renesas,r9a07g044-sysc # RZ/G2{L,LC}
> > - - renesas,r9a07g054-sysc # RZ/V2L
> > + - renesas,r9a07g043-rzfive-sysc # RZ/Five
>
> renesas,r9a07g043f-sysc?
>
Agreed.

> But I'm wondering if we really need a different compatible value?
> It looks like both blocks differ only in external wiring, so if
> anything needs to be handled differently (the removed/added registers
> are related to CPU topology), that can be inferred from the system
> topology (or even #ifdef CONFIG_{ARM64,RISCV} ;-)
>
Good point, but I wonder if we would end up in too many #ifdef
CONFIG_{ARM64,RISCV} checks. If thats OK I will stick with
"renesas,r9a07g043-sysc"

> > + - renesas,r9a07g043-sysc # RZ/G2UL
> > + - renesas,r9a07g044-sysc # RZ/G2{L,LC}
> > + - renesas,r9a07g054-sysc # RZ/V2L
> >
> > reg:
> > maxItems: 1
> >
> > - interrupts:
> > - items:
> > - - description: CA55/CM33 Sleep/Software Standby Mode request interrupt
> > - - description: CA55 Software Standby Mode release request interrupt
> > - - description: CM33 Software Standby Mode release request interrupt
> > - - description: CA55 ACE Asynchronous Bridge Master/Slave interface deny request interrupt
> > + interrupts: true
> >
> > - interrupt-names:
> > - items:
> > - - const: lpm_int
> > - - const: ca55stbydone_int
> > - - const: cm33stbyr_int
> > - - const: ca55_deny
> > + interrupt-names: true
> >
> > required:
> > - compatible
> > - reg
> > - - interrupts
> > - - interrupt-names
> >
> > additionalProperties: false
> >
> > +allOf:
> > + - if:
> > + not:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - renesas,r9a07g043-rzfive-sysc
> > + then:
> > + properties:
> > + interrupts:
> > + items:
> > + - description: CA55/CM33 Sleep/Software Standby Mode request interrupt
> > + - description: CA55 Software Standby Mode release request interrupt
> > + - description: CM33 Software Standby Mode release request interrupt
> > + - description: CA55 ACE Asynchronous Bridge Master/Slave interface deny request interrupt
> > +
> > + interrupt-names:
> > + items:
> > + - const: lpm_int
> > + - const: ca55stbydone_int
> > + - const: cm33stbyr_int
> > + - const: ca55_deny
> > +
> > + required:
> > + - interrupts
> > + - interrupt-names
> > +
> > + else:
> > + properties:
> > + interrupts: false
> > + interrupt-names: false
>
> Do all interrupts{,-names} have to be moved?
> Wouldn't it be sufficient to just have
>
Agreed.

> if [...]
> then:
> required:
> - interrupts
> - interrupt-names
> else:
> properties:
> interrupts: false
> interrupt-names: false
>
> ?
>
> But again, without a new compatible value, you could just make
> interrupts{,-names} not required?
>
You mean we just make it optional for all the SoC's?

Cheers,
Prabhakar

2022-07-22 10:49:05

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: soc: renesas: renesas,rzg2l-sysc: Document RZ/Five SoC

Hi Prabhakar,

On Fri, Jul 22, 2022 at 12:21 PM Lad, Prabhakar
<[email protected]> wrote:
> On Fri, Jul 22, 2022 at 10:31 AM Geert Uytterhoeven
> <[email protected]> wrote:
> > On Fri, Jul 22, 2022 at 12:15 AM Lad Prabhakar
> > <[email protected]> wrote:
> > > Document RZ/Five (R9A07G043) SYSC bindings. SYSC block found on the
> > > RZ/Five SoC is almost identical to one found on the RZ/G2L (and alike)
> > > SoC's. To differentiate RZ/G2UL from RZ/Five, "-rzfive" is included in
> > > the compatible string for the RZ/Five SoC as there are no interrupts
> > > from the SYSC block to the core.
> > >
> > > Signed-off-by: Lad Prabhakar <[email protected]>
> >
> > Thanks for your patch!
> >
> > > ---
> > > .../soc/renesas/renesas,rzg2l-sysc.yaml | 56 +++++++++++++------
> > > 1 file changed, 39 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> > > index ce2875c89329..bdaf05f8b29b 100644
> > > --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> > > +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> > > @@ -20,35 +20,57 @@ description:
> > > properties:
> > > compatible:
> > > enum:
> > > - - renesas,r9a07g043-sysc # RZ/G2UL
> > > - - renesas,r9a07g044-sysc # RZ/G2{L,LC}
> > > - - renesas,r9a07g054-sysc # RZ/V2L
> > > + - renesas,r9a07g043-rzfive-sysc # RZ/Five
> >
> > renesas,r9a07g043f-sysc?
> >
> Agreed.
>
> > But I'm wondering if we really need a different compatible value?
> > It looks like both blocks differ only in external wiring, so if
> > anything needs to be handled differently (the removed/added registers
> > are related to CPU topology), that can be inferred from the system
> > topology (or even #ifdef CONFIG_{ARM64,RISCV} ;-)
> >
> Good point, but I wonder if we would end up in too many #ifdef
> CONFIG_{ARM64,RISCV} checks. If thats OK I will stick with
> "renesas,r9a07g043-sysc"
>
> > > + - renesas,r9a07g043-sysc # RZ/G2UL
> > > + - renesas,r9a07g044-sysc # RZ/G2{L,LC}
> > > + - renesas,r9a07g054-sysc # RZ/V2L
> > >
> > > reg:
> > > maxItems: 1
> > >
> > > - interrupts:
> > > - items:
> > > - - description: CA55/CM33 Sleep/Software Standby Mode request interrupt
> > > - - description: CA55 Software Standby Mode release request interrupt
> > > - - description: CM33 Software Standby Mode release request interrupt
> > > - - description: CA55 ACE Asynchronous Bridge Master/Slave interface deny request interrupt
> > > + interrupts: true
> > >
> > > - interrupt-names:
> > > - items:
> > > - - const: lpm_int
> > > - - const: ca55stbydone_int
> > > - - const: cm33stbyr_int
> > > - - const: ca55_deny
> > > + interrupt-names: true
> > >
> > > required:
> > > - compatible
> > > - reg
> > > - - interrupts
> > > - - interrupt-names
> > >
> > > additionalProperties: false
> > >
> > > +allOf:
> > > + - if:
> > > + not:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + enum:
> > > + - renesas,r9a07g043-rzfive-sysc
> > > + then:
> > > + properties:
> > > + interrupts:
> > > + items:
> > > + - description: CA55/CM33 Sleep/Software Standby Mode request interrupt
> > > + - description: CA55 Software Standby Mode release request interrupt
> > > + - description: CM33 Software Standby Mode release request interrupt
> > > + - description: CA55 ACE Asynchronous Bridge Master/Slave interface deny request interrupt
> > > +
> > > + interrupt-names:
> > > + items:
> > > + - const: lpm_int
> > > + - const: ca55stbydone_int
> > > + - const: cm33stbyr_int
> > > + - const: ca55_deny
> > > +
> > > + required:
> > > + - interrupts
> > > + - interrupt-names
> > > +
> > > + else:
> > > + properties:
> > > + interrupts: false
> > > + interrupt-names: false
> >
> > Do all interrupts{,-names} have to be moved?
> > Wouldn't it be sufficient to just have
> >
> Agreed.
>
> > if [...]
> > then:
> > required:
> > - interrupts
> > - interrupt-names
> > else:
> > properties:
> > interrupts: false
> > interrupt-names: false
> >
> > ?
> >
> > But again, without a new compatible value, you could just make
> > interrupts{,-names} not required?
> >
> You mean we just make it optional for all the SoC's?

Indeed.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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