linux-kernel - Re: [PATCH 1/5] dt-bindings: interrupt-controller: renesas,rzg2l-irqc: Document RZ/Five SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+V-a8vtqUP0M0A-UP1dXBb8Hwejyn39Ah_zd1bRKpEQreuLDw@mail.gmail.com>
Date: Tue, 30 Jan 2024 16:00:29 +0000
From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: Conor Dooley <conor@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, 
	Rob Herring <robh+dt@...nel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, 
	Magnus Damm <magnus.damm@...il.com>, linux-renesas-soc@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-riscv@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, Biju Das <biju.das.jz@...renesas.com>, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH 1/5] dt-bindings: interrupt-controller:
 renesas,rzg2l-irqc: Document RZ/Five SoC

Hi Geert,

On Tue, Jan 30, 2024 at 1:06 PM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, Jan 30, 2024 at 1:59 PM Lad, Prabhakar
> <prabhakar.csengg@...il.com> wrote:
> > On Tue, Jan 30, 2024 at 11:13 AM Geert Uytterhoeven
> > <geert@...ux-m68k.org> wrote:
> > > On Mon, Jan 29, 2024 at 6:30 PM Conor Dooley <conor@...nel.org> wrote:
> > > > On Mon, Jan 29, 2024 at 03:16:14PM +0000, Prabhakar wrote:
> > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...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 differences,
> > > > > * 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/Five
> > > > > SoC.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesascom>
> > >
> > > > > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > > > @@ -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
> > >
> > > Why not use the naming from the docs (all 6 include "ti")?
> > > EC7TIE1_0, EC7TIE2_0, EC7TIOVF_0, EC7TIE1_1, EC7TIE2_1, EC7TIOVF_1
> > > => ec7tie1-0, ec7tie2-0, ec7tiovf-0, ...?
> > >
> > Agreed.
> >
> > > > 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.
> > >
> > > I'm gonna repeat (and extend) my question from [1]: How come we thought
> > > RZ/G2L and RZ/V2L do not have the bus error and ECCRAM interrupts?
> > >
> > Hmm not sure how this was missed earlier.
> >
> > > Looks like most of the conditional handling can be removed (see below).
> > >
> > > > Given the commit message, I figure that providing 48 interrupts for
> > > > (at least some of) those devices would be incorrect?
> > >
> > > Looks like all of RZ/G2L{,C}, RZ/V2L, RZ/G2UL, and RZ/Five support
> > > all 48 interrupts.  RZ/G3S lacks the final three for ECCRAM1.
> > >
> > Agreed for RZ/G2L{,C}, RZ/V2L, RZ/G2UL, and RZ/Five, but for RZ/G3S it
> > becomes tricky the interrupts for ECCRAM0/1 are combined hence they
> > have just 3 interrupts. How do you propose the above interrupt naming?
>
> I guess it doesn't hurt to have an index 0 on a part that has only a
> single set?
>
Let's go with this option...

> Alternatives would be to
>   1. Drop the index completely on RZ/G3S, complicating bindings and
>      driver,
>   1. Drop the index for the first set, and use index 2 for the second set,
>      causing the names to differ even more on parts with 2 sets.
>
..instead of complicating.

Cheers,
Prabhakar