Hi Prabhakar,
On Fri, Jul 22, 2022 at 12:21 PM Lad, Prabhakar
<prabhakar.csengg@xxxxxxxxx> wrote:
> On Fri, Jul 22, 2022 at 10:31 AM Geert Uytterhoeven
> <geert@xxxxxxxxxxxxxx> wrote:
> > 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?
Indeed.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
