linux-kernel - Re: [PATCH 2/2] Input: omap-keypad: Cleanup - remove unnecessary IRQ enabling/disabling

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]

Message-ID: <51ED6AF1.9070605@ti.com>
Date:	Mon, 22 Jul 2013 20:25:05 +0300
From:	Illia Smyrnov <x0194613@...com>
To:	<balbi@...com>
CC:	Illia Smyrnov <illia.smyrnov@...ballogic.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	<linux-input@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-omap@...r.kernel.org>
Subject: Re: [PATCH 2/2] Input: omap-keypad: Cleanup - remove unnecessary
 IRQ enabling/disabling

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/