From: Nava kishore Manne <nava.manne@xilinx.com>
To: Linus Walleij <linus.walleij@linaro.org>
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        =?utf-8?B?QW5kZXJzc29uLCBCasO2cm4=?= 
	<bjorn.andersson@sonymobile.com>,
        "Peng Fan" <van.freenix@gmail.com>,
        Linux Input <linux-input@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [LINUX PATCH v2] gpio_keys: Added support to read the IRQ_FLAGS
 from devicetree
Thread-Topic: [LINUX PATCH v2] gpio_keys: Added support to read the
 IRQ_FLAGS from devicetree
Thread-Index: AQHRjlhAuUGWa1s3i02trkPBr2DnZZ95IcSAgAOurgA=
Date: Wed, 6 Apr 2016 11:32:55 +0000
Message-ID: <C89496FEAE474D468F30D558A9468D9F26CCF95F@XAP-PVEXMBX01.xlnx.xilinx.com>
References: <1459763770-2296-1-git-send-email-navam@xilinx.com>
 <CACRpkdZ9pqgScTV_z9TLyBgB39vuj6SGCSpz2pYj89O6XDnFrA@mail.gmail.com>
In-Reply-To: <CACRpkdZ9pqgScTV_z9TLyBgB39vuj6SGCSpz2pYj89O6XDnFrA@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Apr 2016 11:32:58.5769
 (UTC)
X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.60.83];Helo=[xsj-pvapsmtpgw01]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1NAM02HT091
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u36C6Qxw002257
Content-Length: 3169
Lines: 84

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
> <nava.manne@xilinx.com> wrote:
> 
> > 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
> 
> >                 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