linux-kernel - Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq

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: <CAMuHMdUDts8LWYrftYp0VSxDX8kaP1a9N5YGZDNCAuAsr84EAw@mail.gmail.com>
Date:   Mon, 27 Dec 2021 11:02:32 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     "Lad, Prabhakar" <prabhakar.csengg@...il.com>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Marc Zyngier <maz@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to
 get the interrupt

Hi Andy,

On Mon, Dec 27, 2021 at 10:57 AM Andy Shevchenko
<andy.shevchenko@...il.com> wrote:
> On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
> <geert@...ux-m68k.org> wrote:
> > On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> > <andy.shevchenko@...il.com> wrote:
> > > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > > <prabhakar.csengg@...il.com> wrote:
> > > > On Sat, Dec 25, 2021 at 5:40 PM Andy Shevchenko
> > > > <andy.shevchenko@...il.com> wrote:
> > > > > On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
> > > > > <prabhakar.csengg@...il.com> wrote:
> > > > > > On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> > > > > > <andy.shevchenko@...il.com> wrote:
> > > > > > > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > > > > > > <prabhakar.mahadev-lad.rj@...renesas.com> wrote:
> > > > >
> > > > > > > ret = platform_get_irq_optional(...);
> > > > > > > if (ret < 0 && ret != -ENXIO)
> > > > > > >   return ret;
> > > > > > > if (ret > 0)
> > > > > > >   ...we got it...
> > > > > > >
> > > > > > > It will allow the future API fix of platform_get_irq_optional() to be
> > > > > > > really optional.
> > > > > > >
> > > > > > Later patch [0] (merged into -next) does check for -ENXIO first.
> > > > > >
> > > > > > [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/
> > > > >
> > > > > The problem is that it doesn't consider 0 as no IRQ.
> > > > >
> > > > Can you please point me to the discussion/patch where this API change
> > > > is considered/discussed. Just to clarify now the new API for
> > > > platform_get_irq_optional() will return "0" in case there is no
> > > > interrupt and not not -ENXIO anymore?
> > >
> > > The longest one happened here:
> > > https://lore.kernel.org/linux-ide/20211209145937.77719-1-andriy.shevchenko@linux.intel.com/T/#u
> > >
> > > It has links to some other discussions on the topic.
> > >
> > > > When will this patch be merged for the new api, so that I can base my
> > > > patches on top of it to avoid more changes?
> > >
> > > You can simply imply that, I dunno when it gets merged (from my point
> > > of view the users should be fixed first, and since you are adding
> > > users, the burden is increasing).
> >
> > Not only users (drivers), but also providers (architecture-specific code).
> > IRQ zero is still valid on some architectures, e.g. on SH[1].
>
> Are we talking about vIRQ?
> And users are fine with a big warning?

The warning is only seen when a driver uses platorm_get_irq{,_optional}().
There are several other ways to obtain interrupts, avoiding the
big warning.

> My understanding is that the architecture code there is broken. It
> needs to be fixed to use IRQ domains and all that machinery instead of
> what it does.
>
> 0 is "no IRQ" in Linux.
>
> > [1] https://lore.kernel.org/linux-renesas-soc/CAMuHMdUg3=q7gyaVHP0XcYUOo3PQUUv8Hc8wp5faVQ+bTBpg4A@mail.gmail.com/

BTW, perhaps the IRQ subsystem should switch from integer IRQ numbers
to an opaque object, like was done for GPIO before? The IRQ subsystem
is one of the few (only?) subsystems still using plain integers. That
way we don't need this two-step platform_get_irq_optional() conversion
(step 1: check for both -ENXIO and zero, step 2: drop the check for
-ENXIO).

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