Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220721221212.18491-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <20220721221212.18491-2-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <CAMuHMdVy0w8Y-+MegHnjdFkQ7vB1Y=jPY-9atTFT_WTVYOiGLg@mail.gmail.com> <CA+V-a8tddFVQnJjKSg9MTKB-Hh3X=2OwsU+THis+KFEfz7KB=g@mail.gmail.com>
In-Reply-To: <CA+V-a8tddFVQnJjKSg9MTKB-Hh3X=2OwsU+THis+KFEfz7KB=g@mail.gmail.com>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Fri, 22 Jul 2022 12:24:45 +0200
Message-ID: <CAMuHMdWzakUT4vEV=MSw+tCReXn0mng1XPKf6iE-5S8q-XQjnw@mail.gmail.com>
Subject: Re: [PATCH 1/2] dt-bindings: soc: renesas: renesas,rzg2l-sysc:
 Document RZ/Five SoC
To:     "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc:     Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
        Magnus Damm <magnus.damm@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Biju Das <biju.das.jz@bp.renesas.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Prabhakar,

On Fri, Jul 22, 2022 at 12:21 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Fri, Jul 22, 2022 at 10:31 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Fri, Jul 22, 2022 at 12:15 AM Lad Prabhakar
> > <prabhakar.mahadev-lad.rj@bp.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@bp.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@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