From: Illia Smyrnov <x0194613@ti.com>
To: balbi@ti.com
Cc: Illia Smyrnov <illia.smyrnov@globallogic.com>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-omap@vger.kernel.org
Subject: Re: [PATCH 2/2] Input: omap-keypad: Cleanup - remove unnecessary IRQ enabling/disabling
Date: Mon, 22 Jul 2013 20:25:05 +0300 [thread overview]
Message-ID: <51ED6AF1.9070605@ti.com> (raw)
In-Reply-To: <20130719132648.GB17188@arwen.pp.htv.fi>
Hi,
On 07/19/2013 04:26 PM, Felipe Balbi wrote:
> Hi,
>
> [...]
> please don't remove this code. It'll be good to have this around when we
> move the driver to threaded IRQs without IRQF_ONESHOT. In fact, it would
> be very simple to implement such a change, wanna take it up ?
>
> It should be doable in few patches:
>
> 1) switch over to request_threaded_irq()
>
> just blind move to a thread, without hardirq handler, so
> IRQF_ONESHOT is mandatory.
>
> 2) add hardirq handler
>
> read IRQSTATUS to check if our device has generated IRQs
> returning IRQ_WAKE_THREAD if true
>
> 3) move 'IRQ masking logic' to hardirq handler, before returning
> IRQ_WAKE_THREAD
>
> this will let you remove IRQF_ONESHOT
>
> 4) finally remove IRQF_ONESHOT
>
> this makes sure that IRQs aren't kept disabled until we have
> time to iterate over the entire keypad matrix. Only the keypad
> IRQ will be masked.
>
Ok, but why we need to remove IRQF_ONESHOT flag for omap keypad driver?
The keypad IRQ isn't shared IRQ and in our case hardirq handler will
always return IRQ_WAKE_THREAD like default irq_default_primary_handler
do. With IRQF_ONESHOT flag IRQ line will be masked until the threaded
handler finished, but there is only keypad on this line.
I tested two versions:
the first one - just threaded IRQs with IRQF_ONESHOT and without
specific hardirq handler.
the second version - threaded IRQs without IRQF_ONESHOT as you described.
Both versions was successfully tested on Blaze's keypad.
WARNING: multiple messages have this Message-ID (diff)
From: Illia Smyrnov <x0194613@ti.com>
To: <balbi@ti.com>
Cc: Illia Smyrnov <illia.smyrnov@globallogic.com>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
<linux-input@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linux-omap@vger.kernel.org>
Subject: Re: [PATCH 2/2] Input: omap-keypad: Cleanup - remove unnecessary IRQ enabling/disabling
Date: Mon, 22 Jul 2013 20:25:05 +0300 [thread overview]
Message-ID: <51ED6AF1.9070605@ti.com> (raw)
In-Reply-To: <20130719132648.GB17188@arwen.pp.htv.fi>
Hi,
On 07/19/2013 04:26 PM, Felipe Balbi wrote:
> Hi,
>
> [...]
> please don't remove this code. It'll be good to have this around when we
> move the driver to threaded IRQs without IRQF_ONESHOT. In fact, it would
> be very simple to implement such a change, wanna take it up ?
>
> It should be doable in few patches:
>
> 1) switch over to request_threaded_irq()
>
> just blind move to a thread, without hardirq handler, so
> IRQF_ONESHOT is mandatory.
>
> 2) add hardirq handler
>
> read IRQSTATUS to check if our device has generated IRQs
> returning IRQ_WAKE_THREAD if true
>
> 3) move 'IRQ masking logic' to hardirq handler, before returning
> IRQ_WAKE_THREAD
>
> this will let you remove IRQF_ONESHOT
>
> 4) finally remove IRQF_ONESHOT
>
> this makes sure that IRQs aren't kept disabled until we have
> time to iterate over the entire keypad matrix. Only the keypad
> IRQ will be masked.
>
Ok, but why we need to remove IRQF_ONESHOT flag for omap keypad driver?
The keypad IRQ isn't shared IRQ and in our case hardirq handler will
always return IRQ_WAKE_THREAD like default irq_default_primary_handler
do. With IRQF_ONESHOT flag IRQ line will be masked until the threaded
handler finished, but there is only keypad on this line.
I tested two versions:
the first one - just threaded IRQs with IRQF_ONESHOT and without
specific hardirq handler.
the second version - threaded IRQs without IRQF_ONESHOT as you described.
Both versions was successfully tested on Blaze's keypad.
next prev parent reply other threads:[~2013-07-22 17:25 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-19 13:03 [PATCH 0/2] Input: omap-keypad: Cleanup - remove hardcoded values and IRQ enabling/disabling Illia Smyrnov
2013-07-19 13:03 ` Illia Smyrnov
2013-07-19 13:03 ` [PATCH 1/2] Input: omap-keypad: Cleanup - use bitfiled instead of hardcoded values Illia Smyrnov
2013-07-19 13:03 ` Illia Smyrnov
2013-07-19 13:03 ` [PATCH 2/2] Input: omap-keypad: Cleanup - remove unnecessary IRQ enabling/disabling Illia Smyrnov
2013-07-19 13:03 ` Illia Smyrnov
2013-07-19 13:26 ` Felipe Balbi
2013-07-19 13:26 ` Felipe Balbi
2013-07-22 17:25 ` Illia Smyrnov [this message]
2013-07-22 17:25 ` Illia Smyrnov
2013-07-22 21:04 ` Felipe Balbi
2013-07-22 21:04 ` Felipe Balbi
2013-07-19 13:10 ` [PATCH 0/2] Input: omap-keypad: Cleanup - remove hardcoded values and " Illia Smyrnov
2013-07-19 13:10 ` Illia Smyrnov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51ED6AF1.9070605@ti.com \
--to=x0194613@ti.com \
--cc=balbi@ti.com \
--cc=dmitry.torokhov@gmail.com \
--cc=illia.smyrnov@globallogic.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.