[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMuHMdWzakUT4vEV=MSw+tCReXn0mng1XPKf6iE-5S8q-XQjnw@mail.gmail.com>
Date: Fri, 22 Jul 2022 12:24:45 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
Cc: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
Magnus Damm <magnus.damm@...il.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Biju Das <biju.das.jz@...renesas.com>
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
<prabhakar.csengg@...il.com> wrote:
> On Fri, Jul 22, 2022 at 10:31 AM Geert Uytterhoeven
> <geert@...ux-m68k.org> wrote:
> > On Fri, Jul 22, 2022 at 12:15 AM Lad Prabhakar
> > <prabhakar.mahadev-lad.rj@...renesas.com> 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@...renesas.com>
> >
> > 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@...ux-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
Powered by blists - more mailing lists