Re: [PATCH 2/2] Input: omap-keypad: Cleanup - remove unnecessaryIRQ enabling/disabling

[Illia Smyrnov]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/