Re: IRQF_TIMER | IRQF_SHARED ?

[Andres Salomon <dilinger@queued.net>]

On Mon, 12 Dec 2011 16:41:25 +0100
Jens Rottmann <JRottmann@LiPPERTEmbedded.de> wrote:

> Hi Andres,
> 
> one of our customers tripped over the fact that the MFGPT driver
> won't share its IRQ with anyone else. (MFGPT defaulted to same IRQ as
> audio, MFGPT driver loaded first, audio fails.) *No big deal!* They
> don't actually need MFGPT and will simply disable it. It just made me
> wonder ...
> 
> Why would it be such a bad idea to use IRQF_TIMER | IRQF_SHARED (see
> patch below)? mfgpt_tick() already does properly return IRQ_NONE when
> it feels unresponsible. I tested it with either driver loaded first
> and it seemed to work (well, at least audio worked, don't know how to
> explicitly test cs5535-clockevt).

Just loading cs5535-clockevt should start the periodic timer.  On my
XO-1, IRQ 7 starts firing immediately.


> 
> I thought about latencies of IRQ sharing being unacceptable for a
> timer, but ...
> - If MFGPT is loaded first there is no additional latency, is there?
> Audio recieves its IRQs only as 2nd in list but that's not a problem.
> - If MFGPT is loaded second - well, there is a latency, but without
> sharing the IRQ the driver failed to load at all, so that's still an
> improvement.
> 
> But I did not fail to notice that _none_ of the code in
> drivers/clocksource/ uses IRQF_SHARED, obviously this must be
> deliberate.

Hm, maybe tglx knows?  For my part, I don't think it would be a
problem, but I can imagine the reason for not sharing being clock drift
or something to that effect.


> 
> So, what's so bad about IRQF_TIMER | IRQF_SHARED?
> 
> Any education would be welcome, even if combined with flame. :-)
> 
> Thanks and best regards,
> Jens
> 
> --- linux-3.2-rc4/drivers/clocksource/cs5535-clockevt.c
> +++ shared_mfgpt_irq/drivers/clocksource/cs5535-clockevt.c
> @@ -133,7 +133,7 @@ static irqreturn_t mfgpt_tick(int irq, v
> 
>  static struct irqaction mfgptirq  = {
>  	.handler = mfgpt_tick,
> -	.flags = IRQF_DISABLED | IRQF_NOBALANCING | IRQF_TIMER,
> +	.flags = IRQF_DISABLED | IRQF_NOBALANCING | IRQF_TIMER |
> IRQF_SHARED, .name = DRV_NAME,
>  };
> 
> _
>