Thread (4 messages) 4 messages, 3 authors, 2016-04-06

RE: [LINUX PATCH v2] gpio_keys: Added support to read the IRQ_FLAGS from devicetree

From: Nava kishore Manne <hidden>
Date: 2016-04-06 12:06:16
Also in: lkml

Hi Linus walleij,

	One of Our gpio-controller was supporting only edge rising   interrupts. For that reason I implementing the below logic to read the interrupt trigger level from the DT. If it is wrong could you please provide the pointer to solve this issue?

Regards,
Navakishore.


-----Original Message-----
From: Linus Walleij [mailto:linus.walleij@linaro.org]
Sent: Monday, April 04, 2016 4:38 PM
To: Nava kishore Manne
Cc: Dmitry Torokhov; Andersson, Björn; Nava kishore Manne; Peng Fan;
Linux Input; linux-kernel@vger.kernel.org
Subject: Re: [LINUX PATCH v2] gpio_keys: Added support to read the
IRQ_FLAGS from devicetree

On Mon, Apr 4, 2016 at 11:56 AM, Nava kishore Manne
[off-list ref] wrote:
quoted
This patch adds the support to read the IRQ_FLAGS from the device
instead of hard code the flags in gpio_keys_setup_key().
NACK
quoted
                sw14 {
                        label = "sw14";
                        gpios = <&gpio0 12 1>;
                        /*
                         * Triggering Type:
                         *
                         * 1 - edge rising
                         * 2 - edge falling
                         * 4 - level active high
                         * 8 - level active low
                         *
                         */
You are completely violating the existing GPIO flags from include/dt-
bindings/gpio/gpio.h

As you will see, for a twocell GPIO flags are already clearly defined for 0,1,2
and 3. (Bit 0 & 1).

Further, these IRQ edge/level flags already exist in include/dt-
bindings/interrupt-controller/irq.h
but you should not be using those either, because they do not mix with a
GPIO specifier, it's a bit like oil and water.

The standard GPIO bindings already has
GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW
which makes it pretty clear that a GPIO line marked as GPIO_ACTIVE_HIGH
should trigger either on rising edge or level active high and vice versa.

The only information you could *possibly* lack is whether the IRQ should be
edge or level triggered.

But level triggered GPIO buttons *does* *not* *make*
*sense* *at* *all*.

Think about it:

The IRQ line goes level high or low because a user pressed a button with
his/her thumb. Then that is wired in as a level IRQ. So what are we going to
do? Wait in the interrupt handler until the user removes his/her thumb?

Level IRQs on GPIOs only makes sense for devices off-chip where you can
talk to the device and ACK the interrupt, and in this case "talk" does not
mean wire up a speaker telling the user to remove the thumb from the
button because we have recieved the interrupt, albeit that would be the
real-world analogy.

Please tell us what you are actually trying to solve.

	One of Our gpio-controller was supporting only edge rising   interrupts. For that reason I implementing the below logic to read the interrupt trigger level from the DT. If it is wrong could you please provide the pointer to solve this issue?

Regards,
Navakishore.
Yours,
Linus Walleij
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help