linux-kernel - Re: [PATCH 1/4] pinctrl: stm32: set pin to gpio input when used as interrupt

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: <CACRpkdYZixkJroS-gQxYL6xW47-J9UiNv4hOO9F-1b2eYLr8XQ@mail.gmail.com>
Date:   Mon, 24 Apr 2017 14:36:27 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Alexandre TORGUE <alexandre.torgue@...com>
Cc:     Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Patrice Chotard <patrice.chotard@...com>,
        Paul Gortmaker <paul.gortmaker@...driver.com>,
        Rob Herring <robh+dt@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH 1/4] pinctrl: stm32: set pin to gpio input when used as interrupt

On Fri, Apr 7, 2017 at 5:10 PM, Alexandre TORGUE
<alexandre.torgue@...com> wrote:

> This patch ensures that pin is correctly set as gpio input when it is used
> as an interrupt.
>
> Signed-off-by: Alexandre TORGUE <alexandre.torgue@...com>
(...)

> +static int stm32_gpio_irq_request_resources(struct irq_data *irq_data)
> +{
> +       struct stm32_gpio_bank *bank = irq_data->domain->host_data;
> +       u32 ret;
> +
> +       if (!gpiochip_is_requested(&bank->gpio_chip, irq_data->hwirq)) {
> +               ret = stm32_gpio_request(&bank->gpio_chip, irq_data->hwirq);
> +               if (ret)
> +                       return ret;
> +       }

This is wrong. You should only use gpiochip_lock_as_irq(), because of the
following in Documentation/gpio/driver.txt:

---------------
It is legal for any IRQ consumer to request an IRQ from any irqchip no matter
if that is a combined GPIO+IRQ driver. The basic premise is that gpio_chip and
irq_chip are orthogonal, and offering their services independent of each
other.
(...)
So always prepare the hardware and make it ready for action in respective
callbacks from the GPIO and irqchip APIs. Do not rely on gpiod_to_irq() having
been called first.

This orthogonality leads to ambiguities that we need to solve: if there is
competition inside the subsystem which side is using the resource (a certain
GPIO line and register for example) it needs to deny certain operations and
keep track of usage inside of the gpiolib subsystem. This is why the API
below exists.

Locking IRQ usage
-----------------
Input GPIOs can be used as IRQ signals. When this happens, a driver is requested
to mark the GPIO as being used as an IRQ:

        int gpiochip_lock_as_irq(struct gpio_chip *chip, unsigned int offset)

This will prevent the use of non-irq related GPIO APIs until the GPIO IRQ lock
is released:

        void gpiochip_unlock_as_irq(struct gpio_chip *chip, unsigned int offset)

When implementing an irqchip inside a GPIO driver, these two functions should
typically be called in the .startup() and .shutdown() callbacks from the
irqchip.

When using the gpiolib irqchip helpers, these callback are automatically
assigned.
--------------

It is because of easy to make errors like this that I prefer that people try
to use GPIOLIB_IRQCHIP helpers insteaf of rolling their own irqchip code.

Yours,
Linus Walleij