linux-kernel - Re: [PATCH v3] pinctrl: Add SX150X GPIO Extender Pinctrl Driver

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]

Date:   Sun, 23 Oct 2016 12:47:02 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Andrey Smirnov <andrew.smirnov@...il.com>
Cc:     Neil Armstrong <narmstrong@...libre.com>,
        Peter Rosin <peda@...ntia.se>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Roland Stigge <stigge@...com.de>,
        Vladimir Zapolskiy <vladimir_zapolskiy@...tor.com>
Subject: Re: [PATCH v3] pinctrl: Add SX150X GPIO Extender Pinctrl Driver

On Sun, Oct 23, 2016 at 4:50 AM, Andrey Smirnov
<andrew.smirnov@...il.com> wrote:
> On Fri, Oct 21, 2016 at 2:09 AM, Neil Armstrong <narmstrong@...libre.com> wrote:
>> Since the I2C sx150x GPIO expander driver uses platform_data to manage
>> the pins configurations, rewrite the driver as a pinctrl driver using
>> pinconf to get/set pin configurations from DT or debugfs.
>>
>> The pinctrl driver is functionnally equivalent as the gpio-only driver
>> and can use DT for pinconf. The platform_data confirmation is dropped.
>>
>> This patchset removed the gpio-only driver and selects the Pinctrl driver
>> config instead. This patchset also migrates the gpio dt-bindings to pinctrl
>> and add the pinctrl optional properties.
>>
>> The driver was tested with a SX1509 device on a BeagleBone black with
>> interrupt support and on an X86_64 machine over an I2C to USB converter.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@...libre.com>
(...)

>> +               ret = gpiochip_irqchip_add(&pctl->gpio,
>> +                                          &pctl->irq_chip, 0,
>> +                                          handle_edge_irq, IRQ_TYPE_EDGE_BOTH);
>
> Adding irqchip with IRQ_TYPE_EDGE_BOTH triggers a WARN in
> drivers/gpio/gpiolib.c:1671, on a custom Vybrid board that I have with
> the chip, so maybe it should be replaced with IRQ_TYPE_NONE? That's
> what I did for my testing and with that change

I fixed this up when applying. It is corect, it should always be IRQ_TYPE_NONE
unless it is a very old driver using boardfiles. The proper type is set up
when the driver using it requests the IRQ.

Some drivers also need to set the default handler to handle_bad_irq
and then also set that up in the irqchip .set_type() callback, especially
those hardwares that have an ACK register for edge IRQs so that a second
edge irq can come in when handling a first edge IRQ. Level IRQs don't
have this problem for obvious reasons, so we need to select between
handle_edge_irq() or handle_level_irq() on these hardwares.

Could you or Neil or both check if this applies to sx150x?

> Tested-by: Andrey Smirnov <andrew.smirnov@...il.com>

Thanks a lot Andrey!

Yours,
Linus Walleij