Hi Prabhakar, On Tue, Jan 30, 2024 at 1:59 PM Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote: > On Tue, Jan 30, 2024 at 11:13 AM Geert Uytterhoeven > <geert@xxxxxxxxxxxxxx> wrote: > > On Mon, Jan 29, 2024 at 6:30 PM Conor Dooley <conor@xxxxxxxxxx> wrote: > > > On Mon, Jan 29, 2024 at 03:16:14PM +0000, Prabhakar wrote: > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > > > > > > 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@xxxxxxxxxxxxxx> > > > > > > --- 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? 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. 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