linux-kernel - Re: [PATCH 4/9] pinctrl: meson: allow gpio to request 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: <7hk2cvrtxt.fsf@baylibre.com>
Date:   Wed, 26 Oct 2016 08:50:06 -0700
From:   Kevin Hilman <khilman@...libre.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Jerome Brunet <jbrunet@...libre.com>,
        Marc Zyngier <marc.zyngier@....com>,
        Carlo Caione <carlo@...one.org>,
        "open list\:ARM\/Amlogic Meson..." 
        <linux-amlogic@...ts.infradead.org>,
        "linux-arm-kernel\@lists.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-gpio\@vger.kernel.org" <linux-gpio@...r.kernel.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree\@vger.kernel.org" <devicetree@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Rob Herring <robh+dt@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH 4/9] pinctrl: meson: allow gpio to request irq

Hi Linus,

Linus Walleij <linus.walleij@...aro.org> writes:

> On Wed, Oct 26, 2016 at 4:22 PM, Jerome Brunet <jbrunet@...libre.com> wrote:
>> On Tue, 2016-10-25 at 20:20 +0200, Linus Walleij wrote:
>
>>> However the semantic is such, that it is not necessary to call
>>> to_irq()
>>> before using an IRQ: the irqchip and gpiochip abstractions should be
>>> orthogonal.
>>
>> Linus,
>>
>> They are orthogonal. You can request an irq from the irqchip controller
>> without the gpiochip, like any other irq controller.
>
> OK good, sorry if I'm stating the obvious.
>
>> irq_create_mapping (and irq_create_fwspec_mapping) internally calls
>> irq_find_mapping. So if the mapping already exist (the irq is already
>> used before calling to_irq), the existing mapping will be returned. The
>> mapping will be actually created only if needed. It seems to be in line
>> with your explanation, no ?
>
> Yes, but you want to call irq_create_mapping() in slowpath (irq setup)
> and irq_find_mapping() in fastpath (irq handler). Else the first IRQ
> may result in unwelcomed surprises.
>
>> There is really a *lot* of gpio drivers which use irq_create_mapping in
>> the to_irq callback, are these all wrong ?
>
> Yes they are all wrong. They should all be using irq_find_mapping().

So, dumb question from someone trying (but having a hard time) to follow
and understand the rationale...

If it's wrong enough to completely reject, why are changes still being
merged that are doing it so wrong?  (e.g. like this one[1], just merged
for v4.9)

Kevin

[1] 0eb9f683336d pinctrl: Add IRQ support to STM32 gpios
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/pinctrl/stm32/pinctrl-stm32.c?id=0eb9f683336d7eb99a3b75987620417c574ffb57