Received-SPF: pass (google.com: domain of linux-kernel+bounces-49565-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240129151618.90922-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <20240129151618.90922-2-prabhakar.mahadev-lad.rj@bp.renesas.com> <20240129-magical-unclaimed-e725e2491ccb@spud>
In-Reply-To: <20240129-magical-unclaimed-e725e2491ccb@spud>
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Date: Fri, 2 Feb 2024 09:22:06 +0000
Message-ID: <CA+V-a8vQdT+Rav9SNUTE7iFP2xWh+dJ=ZnQRhBpXaQDHOfVEdQ@mail.gmail.com>
Subject: Re: [PATCH 1/5] dt-bindings: interrupt-controller:
 renesas,rzg2l-irqc: Document RZ/Five SoC
To: Conor Dooley <conor@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>, Geert Uytterhoeven <geert+renesas@glider.be>, 
	Rob Herring <robh+dt@kernel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
	Magnus Damm <magnus.damm@gmail.com>, linux-renesas-soc@vger.kernel.org, 
	devicetree@vger.kernel.org, linux-riscv@lists.infradead.org, 
	linux-kernel@vger.kernel.org, Biju Das <biju.das.jz@bp.renesas.com>, 
	Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Conor,

Thank you for the review.

On Mon, Jan 29, 2024 at 5:30=E2=80=AFPM Conor Dooley <conor@kernel.org> wro=
te:
>
> On Mon, Jan 29, 2024 at 03:16:14PM +0000, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Document RZ/Five (R9A07G043F) IRQC bindings. The IRQC block on RZ/Five =
SoC
> > is almost identical to one found on the RZ/G2L SoC with below differenc=
es,
> > * Additional BUS error interrupt
> > * Additional ECCRAM error interrupt
> > * Has additional mask control registers for NMI/IRQ/TINT
> >
> > Hence new compatible string "renesas,r9a07g043f-irqc" is added for RZ/F=
ive
> > SoC.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  .../renesas,rzg2l-irqc.yaml                   | 27 +++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/ren=
esas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controll=
er/renesas,rzg2l-irqc.yaml
> > index d3b5aec0a3f7..3abc01e48934 100644
> > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rz=
g2l-irqc.yaml
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rz=
g2l-irqc.yaml
> > @@ -23,6 +23,7 @@ properties:
> >    compatible:
> >      items:
> >        - enum:
> > +          - renesas,r9a07g043f-irqc   # RZ/Five
> >            - renesas,r9a07g043u-irqc   # RZ/G2UL
> >            - renesas,r9a07g044-irqc    # RZ/G2{L,LC}
> >            - renesas,r9a07g054-irqc    # RZ/V2L
> > @@ -88,6 +89,12 @@ properties:
> >        - description: GPIO interrupt, TINT30
> >        - description: GPIO interrupt, TINT31
> >        - description: Bus error interrupt
> > +      - description: ECCRAM0 TIE1 interrupt
> > +      - description: ECCRAM0 TIE2 interrupt
> > +      - description: ECCRAM0 overflow interrupt
> > +      - description: ECCRAM1 TIE1 interrupt
> > +      - description: ECCRAM1 TIE2 interrupt
> > +      - description: ECCRAM1 overflow interrupt
> >
> >    interrupt-names:
> >      minItems: 41
> > @@ -134,6 +141,12 @@ properties:
> >        - const: tint30
> >        - const: tint31
> >        - const: bus-err
> > +      - const: eccram0-tie1
> > +      - const: eccram0-tie2
> > +      - const: eccram0-ovf
> > +      - const: eccram1-tie1
> > +      - const: eccram1-tie2
> > +      - const: eccram1-ovf
>
> I think the restrictions already in the file become incorrect with this
> patch:
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - renesas,r9a07g043u-irqc
>               - renesas,r9a08g045-irqc
>     then:
>       properties:
>         interrupts:
>           minItems: 42
>         interrupt-names:
>           minItems: 42
>       required:
>         - interrupt-names
>
> This used to require all 42 interrupts for the two compatibles here
> and at least the first 41 otherwise. Now you've increased the number of
> interrupts to 48 thereby removing the upper limits on the existing
> devices.
>
> Given the commit message, I figure that providing 48 interrupts for
> (at least some of) those devices would be incorrect?
>
Agreed, I will fix this in the next version.

Cheers,
Prabhakar