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: <20221221000242.340202-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <20221221000242.340202-5-prabhakar.mahadev-lad.rj@bp.renesas.com> <86o7rxawhn.wl-maz@kernel.org>
In-Reply-To: <86o7rxawhn.wl-maz@kernel.org>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Wed, 21 Dec 2022 13:18:08 +0100
Message-ID: <CAMuHMdX++dbcsxyGRqiRzgukeU4aoAi3hDV5zyfH1s11dFba-A@mail.gmail.com>
Subject: Re: [PATCH v2 4/9] irqchip: irq-renesas-rzg2l: Add support for
 RZ/G2UL SoC
To:     Marc Zyngier <maz@kernel.org>
Cc:     Prabhakar <prabhakar.csengg@gmail.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Magnus Damm <magnus.damm@gmail.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
        Biju Das <biju.das.jz@bp.renesas.com>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Dec 21, 2022 at 11:20 AM Marc Zyngier <maz@kernel.org> wrote:
> On Wed, 21 Dec 2022 00:02:37 +0000,
> Prabhakar <prabhakar.csengg@gmail.com> wrote:
> >
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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@bp.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

> > +                     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".

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