linux-kernel - Re: gpiochip_lock_as_irq on pins without FLAG

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: <CACRpkdbU2hs9VVH+sCNVi2hoB53WJ6LmUdndOvbjk9wVvyf4kQ@mail.gmail.com>
Date:   Fri, 2 Jul 2021 02:24:51 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Vincent Pelletier <plr.vincent@...il.com>
Cc:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or
 feature ?

On Tue, Jun 29, 2021 at 12:52 AM Vincent Pelletier
<plr.vincent@...il.com> wrote:

> While on this topic, there is a bullet point in
> Documentation/driver-api/gpio/driver.rst which I fail to understand:
>
> | - Nominally set all handlers to handle_bad_irq() in the setup call and pass
> |   handle_bad_irq() as flow handler parameter in
> gpiochip_irqchip_add() if it is
> |   expected for GPIO driver that irqchip .set_type() callback will be called
> |   before using/enabling each GPIO IRQ. Then set the handler to
> |   handle_level_irq() and/or handle_edge_irq() in the irqchip .set_type()
> |   callback depending on what your controller supports and what is requested
> |   by the consumer.
>
> - why the plural in "set all handlers to handle_bad_irq()" ? Isn't
>   there only a single handler in struct gpio_irq_chip ?

girq->handler will be applied (inside gpiolib) successively to
all the irq lines on the irqchip, as many as you have GPIO lines.
It will then be set to something usable when we get
to ->set_type() when an IRQ is being requested.

> - I do not find a function named gpiochip_irqchip_add(), only
>   gpiochip_irqchip_add_domain()

This refers to the old API that didn't add the irqchip with the
gpiochip. Needs updating. Patches welcome ;)

> - "Then set the handler to [...] in the irqchip .set_type() callback"
>   Isn't set_type per-pin, and isn't the interrupt handler chip-level ?

Those are two different things. The ->set_type() callback is
per-line (call it lines, not pins, because terminology can be
confusing otherwise), these are the interrupt handlers per-line
apart from of course the handler the consumer provides in
e.g. request_[threaded_]irq().

There is usually another cascading interrupt handler for the GPIO
irqchip itself, usually what is passed in girq->parent_handler
*BUT* you are looking at an hierarchical interrupt controller with
one line per gpio line to the next level of irq controller, so
you do not see this. You don't have a chip-level interrupt
handler in the sifive driver, all falls through upward in the
hierarchy to the next overarching interrupt controller.

Yours,
Linus Walleij