[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 18/18] xen/arm: IRQ: Handle multiple action per IRQ
On 04/16/2014 04:54 PM, Ian Campbell wrote: > On Tue, 2014-04-08 at 15:44 +0100, Julien Grall wrote: >> desc->status &= ~IRQ_PENDING; >> spin_unlock_irq(&desc->lock); >> - action->handler(irq, action->dev_id, regs); >> + list_for_each_entry_safe(action, next, &desc->action, next) >> + action->handler(irq, action->dev_id, regs); > > You aren't removing entries from within the loop so I don't think you > need the _safe variant. As we release the desc->lock here, it might be possible to have the list changed under the CPU feet by release_irq. With the double-linked list, how do we make sure that it won't happen? >> spin_lock_irq(&desc->lock); >> } >> >> @@ -217,33 +220,69 @@ void release_irq(unsigned int irq, const void *dev_id) >> { >> struct irq_desc *desc; >> unsigned long flags; >> - struct irqaction *action; >> + struct irqaction *action; >> + bool_t found = 0; >> >> desc = irq_to_desc(irq); >> >> spin_lock_irqsave(&desc->lock,flags); >> >> - desc->handler->shutdown(desc); >> + list_for_each_entry(action, &desc->action, next) >> + { >> + if ( action->dev_id == dev_id ) >> + { >> + found = 1; > > Extra space before =. > > I actually think a goto found would actually be clearer here than the > flag. I'm not a big fan of goto. Anyway, I will use it here if you think it's clearer. > for each > if (got it) > goto found > > printk(not found) > return > > found: > /* Found it. */ >> + /* Sanity checks: >> + * - if the IRQ is marked as shared >> + * - dev_id is not NULL when IRQF_SHARED is set >> + */ >> + if ( !list_empty(&desc->action) && >> + (!(desc->status & IRQF_SHARED) || !shared) ) >> + return -EINVAL; > > Did you mean EBUSY? Right, EBUSY would be better here. >> @@ -68,7 +72,11 @@ typedef struct irq_desc { >> unsigned int status; /* IRQ status */ >> hw_irq_controller *handler; >> struct msi_desc *msi_desc; >> +#ifdef CONFIG_IRQ_HAS_MULTIPLE_ACTION >> + struct list_head action; /* IRQ action list */ >> +#else >> struct irqaction *action; /* IRQ action list */ > > This was never actually a list I think, and the comment is certainly > wrong now. I guess it was copied from Linux :). I will change the comment into "IRQ action" Regards, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |