lkml.org 
[lkml]   [2010]   [Oct]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC IRQ] genirq: fix handle_nested_irq for lazy disable
Thomas Gleixner wrote:
> On Thu, 28 Oct 2010, Abhijeet Dharmapurikar wrote:
>> Thomas Gleixner wrote:
>>> Aside of that this wont work for edge triggered interrupts, as you'd
>>> loose the edge, so this needs more thought and a thorough look at the
>>> users of handle_nested_irq().
>> I didn't understand this though. This patch will mask the interrupt in the
>> controller even if it were edge. My interrupt controller latches
>> edges and wants a mask (or an ack) to be executed to deactivate the line
>> summary line. Do you mean that I should mark the interrupt IRQ_PENDING if it
>> were an edge before masking it? If not, can you please explain.
>
> See handle_edge_irq().
>
> An edge is a one time event. Once you mask/ack it, it's gone. So now
> when you unmask it won't reissue the interrupt on the hardware
> level. Level interrupts do, as the mask does not affect that.

handle_edge_irq() needs to handle nested invocations and so it checks if
the irq is in progress and does the right masking/unmasking and calling
the handler again.
But I think we will never be executing an interrupt within an interrupt
in the threaded irq controller case. By that I mean the irq controller
thread wont call handle_nested_irq on the same interrupt until the first
execution finishes. We don't need to worry about nested calls to
handle_nested_irq().
If this is right, simply marking the edge interrupt IRQF_PENDING before
masking it will suffice IMO.
Please let me know if this is closer to what you were suggesting?

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index baa5c4a..2dd0228 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -419,6 +419,7 @@ void handle_nested_irq(unsigned int irq)
{
struct irq_desc *desc = irq_to_desc(irq);
struct irqaction *action;
+ int mask_this_irq = 0;
irqreturn_t action_ret;

might_sleep();
@@ -428,8 +429,12 @@ void handle_nested_irq(unsigned int irq)
kstat_incr_irqs_this_cpu(irq, desc);

action = desc->action;
- if (unlikely(!action || (desc->status & IRQ_DISABLED)))
+ if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
+ mask_this_irq = 1;
+ if (!(desc->status & IRQ_LEVEL))
+ desc->status |= IRQ_PENDING;
goto out_unlock;
+ }

desc->status |= IRQ_INPROGRESS;
raw_spin_unlock_irq(&desc->lock);
@@ -443,6 +448,11 @@ void handle_nested_irq(unsigned int irq)

out_unlock:
raw_spin_unlock_irq(&desc->lock);
+ if (unlikely(mask_this_irq)) {
+ chip_bus_lock(desc);
+ mask_irq(irq);
+ chip_bus_sync_unlock(desc);
+ }
}
EXPORT_SYMBOL_GPL(handle_nested_irq);

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

\
 
 \ /
  Last update: 2010-10-29 02:07    [W:0.144 / U:0.260 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site