On 2013-03-27 00:05, Gilles Chanteperdrix wrote:
> 
> Hi Jan,
> 
> I seem to recall you sent patches to fix threaded irqs some time ago. I 

In fact, that was Wolfgang with commits fdda86bca9 and 54d161b85b for
2.6.38 back then.

> am having a problem here without SMP, whereas the system works with 
> SMP. A fasteoi irq handler triggers repeatedly while its threaded 
> counterpart never gets triggered. I believe this is because 
> handle_fasteoi_irq unconditionally releases the irq line.

You mean it doesn't respect the conditions in cond_unmask_irq?

> The following 
> patch seems to fix the issue though I would prefer a cleaner solution.
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 11e75d1..2ff8d3a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -463,8 +463,11 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc 
> *desc)
>  
>  #ifdef CONFIG_IPIPE
>       /* XXX: IRQCHIP_EOI_IF_HANDLED is ignored. */
> -     if (desc->irq_data.chip->irq_release)
> -             desc->irq_data.chip->irq_release(&desc->irq_data);
> +     if (desc->irq_data.chip->irq_release) {
> +             if ((!(desc->istate & IRQS_ONESHOT) ||
> +                  !desc->threads_oneshot))
> +                     desc->irq_data.chip->irq_release(&desc->irq_data);
> +     }
>  out_eoi:
>  #else  /* !CONFIG_IPIPE */
>       if (desc->istate & IRQS_ONESHOT)
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index e49a288..485c2c4 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -715,9 +715,15 @@ again:
>  
>       desc->threads_oneshot &= ~action->thread_mask;
>  
> -     if (!desc->threads_oneshot && !irqd_irq_disabled(&desc->irq_data) &&
> -         irqd_irq_masked(&desc->irq_data))
> -             unmask_irq(desc);
> +     if (!desc->threads_oneshot && !irqd_irq_disabled(&desc->irq_data)) {
> +#ifdef CONFIG_IPIPE
> +             if (desc->irq_data.chip->irq_release)
> +                     desc->irq_data.chip->irq_release(&desc->irq_data);
> +             else 
> +#endif
> +                     if (irqd_irq_masked(&desc->irq_data))
> +                             unmask_irq(desc);
> +     }
>  
>  out_unlock:
>       raw_spin_unlock_irq(&desc->lock);
> 
> 

So this basically extends I-pipe's held phase of the IRQ to the threaded
handler if we are working in oneshot mode, right?

I will have to dig into this again, but without remembering all details,
there are some things that do not look OK:
 - Shouldn't we just replace unmask_irq with irq_release for the I-pipe
   case? IOW: There is too much code under #ifdef CONFIG_IPIPE.
 - It seems we do not respect masking done by Linux, rather
   unconditionally release (AKA unmask) the line.

The latter was right in 2.6.38 but apparently got broken during to port
from 3.2 to 3.4.

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: 
<http://www.xenomai.org/pipermail/xenomai/attachments/20130327/d590dc7c/attachment.pgp>
_______________________________________________
Xenomai mailing list
[email protected]
http://www.xenomai.org/mailman/listinfo/xenomai

Reply via email to