440GX interrupt

[Eugene Surovegin]

On Thu, Feb 17, 2005 at 03:50:27PM -0600, Barbier, Renaud (GE Infrastructure) wrote:
> I have a question regarding interrupt and irq locking.
> I derived  (or copied from somehwere)a library (linux 2.4.26) for the 440GX from ppc4xx_pic.c to take care of the last interrupt register (UIC2).
> This a newbie question regarding get_irq/spin_lock
> here is get_irq:
> ...
>      bits = mfdcr(DCRN_UIC_MSR(UICBASE));
>      if ((bits & 0x40000000) == 0x40000000)
>      {
>         bits = mfdcr(DCRN_UIC_MSR(UIC0));
>         irq = ( ffs(bits));
>         irq = 32-irq;
> 	}
> 	...
> 
> my question is what guarantee that the code is executed atomically?

ppc_md.get_irq is called with hard irqs disabled, this makes 
its execution context atomic.

> The reason I asked is that we have a driver that did the following in the ioctl call:
> 
> 	disable_irq(26);
> 	/* do something */
> 	enable_irq(26);
> 
> as you noticed there is not any spin_lock.
> Sometimes, this leads  get_irq to see UICBASE indicating an irq in UIC0 and
> UIC0_MSR to return 0. hence you get irq 32 (MAL_SERR) and an infinite loop.

> My current fix is to use irqsave/irqrestore in the driver which I 
> think is the correct way to do (but I may be wrong please help).

Yes, this is preferable to disable/enable_irq.

> However, I have a colleague (here is the human problem of my 
> questions: him or me is the problem) that insists that I should do 
> something in get_irq to have atomic execution.

No, it's already atomic. 

Probably it's a race which cannot be avoided anyway because external 
IRQs are completely async, and your version of ppc4xx_pic.c just has a 
bug. I'll think about it a little more.

Could you try 2.6 version of ppc_4xx_pic.c? I don't think 2.4 has any 
official support for UIC2 anyway.

--
Eugene.