Hi Geert,
Thank you for the review.
On Fri, Jul 22, 2022 at 10:31 AM Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On Fri, Jul 22, 2022 at 12:15 AM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> 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 <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> 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
