[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMz4kuLNbsZjDPW19V96yL3DzY-bJsjRR=o+w0x4PGyxZFZ0dg@mail.gmail.com>
Date: Thu, 1 Mar 2018 15:35:23 +0800
From: Baolin Wang <baolin.wang@...aro.org>
To: Arnd Bergmann <arnd@...db.de>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
gregkh <gregkh@...uxfoundation.org>,
stephen lu <lumotuwe@...il.com>,
Arvind Yadav <arvind.yadav.cs@...il.com>,
Joseph Lo <josephl@...dia.com>,
Kate Stewart <kstewart@...uxfoundation.org>,
Philippe Ombredanne <pombredanne@...b.com>,
Thomas Gleixner <tglx@...utronix.de>,
"open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
DTML <devicetree@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Mark Brown <broonie@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH v3] Input: gpio_keys: Add level trigger support for GPIO keys
On 28 February 2018 at 22:44, Arnd Bergmann <arnd@...db.de> wrote:
> On Wed, Feb 28, 2018 at 1:44 PM, Baolin Wang <baolin.wang@...aro.org> wrote:
>> On some platforms (such as Spreadtrum platform), the GPIO keys can only
>> be triggered by level type. So this patch introduces one property to
>> indicate if the GPIO trigger type is level trigger or edge trigger.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@...aro.org>
>> ---
>> Changes since v2:
>> - Use 'interrupt' property to indicate the irq type.
>>
>> Changes since v1:
>> - Diable the GPIO irq until reversing the GPIO level type.
>
> I've looked at your patch in more detail now, and given it a bit more thought.
>
> I wonder if you could move that logic into your gpiochip/irqchip driver instead.
> It seems that what you do in the gpio-keys driver is to emulate edge triggered
> behavior on a level triggered irqchip.
>
> If you put the same logic into the gpio driver, you could simply make it
> pretend to support an edge trigger on both edges and call into the interrupt
> handler whenever the state changes.
>
That is really a good suggestion, which can avoid duplicate level
reverse logic in different drivers. So this patch can be simplified
just adding one trigger_type to indicate the interrupt type (not
always edge trigger). Thanks for your suggestion.
--
Baolin.wang
Best Regards
Powered by blists - more mailing lists