linux-kernel - Re: [PATCH v2 4/9] irqchip: irq-renesas-rzg2l: Add support for RZ/G2UL 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: <CAMuHMdV0wKGk+EvTfCZJu0z_HYy7YaLLNya4z6=coASsbnkVBQ@mail.gmail.com>
Date:   Thu, 22 Dec 2022 13:51:41 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     "Lad, Prabhakar" <prabhakar.csengg@...il.com>
Cc:     Marc Zyngier <maz@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Magnus Damm <magnus.damm@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-renesas-soc@...r.kernel.org, linux-gpio@...r.kernel.org,
        Biju Das <biju.das.jz@...renesas.com>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v2 4/9] irqchip: irq-renesas-rzg2l: Add support for
 RZ/G2UL SoC

Hi Prabhakar,

On Thu, Dec 22, 2022 at 12:50 PM Lad, Prabhakar
<prabhakar.csengg@...il.com> wrote:
> On Wed, Dec 21, 2022 at 12:18 PM Geert Uytterhoeven
> <geert@...ux-m68k.org> wrote:
> > On Wed, Dec 21, 2022 at 11:20 AM Marc Zyngier <maz@...nel.org> wrote:
> > > On Wed, 21 Dec 2022 00:02:37 +0000,
> > > Prabhakar <prabhakar.csengg@...il.com> wrote:
> > > >
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > > >
> > > > The IRQC block on RZ/G2UL SoC is almost identical to one found on the
> > > > RZ/G2L SoC the only difference being it can support BUS_ERR_INT for
> > > > which it has additional registers.
> > > >
> > > > This patch adds a new entry for "renesas,rzg2ul-irqc" compatible string
> > > > and now that we have interrupt-names property the driver code parses the
> > > > interrupts based on names and for backward compatibility we fallback to
> > > > parse interrupts based on index.
> > > >
> > > > For now we will be using rzg2l_irqc_init() as a callback for RZ/G2UL SoC
> > > > too and in future when the interrupt handler will be registered for
> > > > BUS_ERR_INT we will have to implement a new callback.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> >
> > > > +/* Parse hierarchy domain interrupts ie only IRQ0-7 and TINT0-31 */
> > > > +static int rzg2l_irqc_parse_hierarchy_interrupts(struct rzg2l_irqc_priv *priv,
> > > > +                                              struct device_node *np)
> > > > +{
> > > > +     struct property *pp;
> > > >       unsigned int i;
> > > >       int ret;
> > > >
> > > > +     /*
> > > > +      * first check if interrupt-names property exists if so parse them by name
> > > > +      * or else parse them by index for backward compatibility.
> > > > +      */
> > > > +     pp = of_find_property(np, "interrupt-names", NULL);
> > > > +     if (pp) {
> > > > +             char *irq_name;
> > > > +
> > > > +             /* parse IRQ0-7 */
> > > > +             for (i = 0; i < IRQC_IRQ_COUNT; i++) {
> > > > +                     irq_name = kasprintf(GFP_KERNEL, "irq%d", i);
> >
> > %u
> >
> Ok.
>
> > > > +                     if (!irq_name)
> > > > +                             return -ENOMEM;
> > > > +
> > > > +                     ret = rzg2l_irqc_parse_interrupt_by_name_to_fwspec(priv, np, irq_name, i);
> > >
> > > Am I the only one that find it rather odd to construct a name from an
> > > index, only to get another index back?
> >
> > The issue is that there are two number ranges ("irq%u" and "tint%u"),
> > stored in a single interrupts property.
> >
> > An alternative solution would be to get rid of the "interrupt-names",
> > and use two separate prefixed interrupts properties instead, like is
> > common for e.g. gpios: "irq-interrupts" and "tint-interrupts".
> >
> Maybe I will read all the interrupts based on index only for all the
> SoCs and we still add interrupt-names in dt bindings with the
> dt_binding check we can make sure all the interrupts for each SoC
> exist in the DT and the driver still reads them based on index. Does
> that sound good?

Sure, sounds fine.

You can postpone parsing interrupt-names in the driver (until a new
SoC arrives that uses a different number of IRQ or TINT interrupts).

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