Patchwork fix the interrupt loss problem on powerpc IPIC (2.6.23)

login
register
mail settings
Submitter dayu@datangmobile.cn
Date Feb. 17, 2009, 12:44 p.m.
Message ID <D728AD1FA2543948B89DE29C5BF4CD0716AC2DDE@bjmail1.bj.datangmobile.com>
Download mbox | patch
Permalink /patch/23259/
State Superseded, archived
Headers show

Comments

dayu@datangmobile.cn - Feb. 17, 2009, 12:44 p.m.
From: Da Yu <dayu@datangmobile.cn>
Date: Tue, 17 Feb 2009 19:58:20 +0800
Subject: [PATCH] fix the interrupt loss problem on powerpc IPIC (2.6.23)

Signed-off-by: Da Yu <dayu@datangmobile.cn>
---


  */
 #include <linux/kernel.h>
 #include <linux/init.h>
@@ -561,8 +562,7 @@
 
 	spin_lock_irqsave(&ipic_lock, flags);
 
-	temp = ipic_read(ipic->regs, ipic_info[src].pend);
-	temp |= (1 << (31 - ipic_info[src].bit));
+	temp = (1 << (31 - ipic_info[src].bit));


Remove unneeded brackets.


 	ipic_write(ipic->regs, ipic_info[src].pend, temp);
 
 	spin_unlock_irqrestore(&ipic_lock, flags); @@ -581,8 +581,7 @@
 	temp &= ~(1 << (31 - ipic_info[src].bit));
 	ipic_write(ipic->regs, ipic_info[src].mask, temp);
 
-	temp = ipic_read(ipic->regs, ipic_info[src].pend);
-	temp |= (1 << (31 - ipic_info[src].bit));
+	temp = (1 << (31 - ipic_info[src].bit));

Same as above.


 	ipic_write(ipic->regs, ipic_info[src].pend, temp);
 
 	spin_unlock_irqrestore(&ipic_lock, flags);
Yang Li - Feb. 17, 2009, 2:38 p.m.
On Tue, Feb 17, 2009 at 10:12 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
>
> On Feb 17, 2009, at 6:44 AM, <dayu@datangmobile.cn> <dayu@datangmobile.cn>
> wrote:
>
>> From: Da Yu <dayu@datangmobile.cn>
>> Date: Tue, 17 Feb 2009 19:58:20 +0800
>> Subject: [PATCH] fix the interrupt loss problem on powerpc IPIC (2.6.23)
>>
>> Signed-off-by: Da Yu <dayu@datangmobile.cn>
>> ---
>
> Please provide a bit more description as to why this fixes the issue.

The pending register is write 1 clear.  If there are more than one
external interrupts pending at the same time, acking the first
interrupt will also clear other interrupt pending bits.  That will
cause loss of interrupt.

- Leo
Kumar Gala - Feb. 17, 2009, 2:43 p.m.
On Feb 17, 2009, at 8:38 AM, Li Yang wrote:

> On Tue, Feb 17, 2009 at 10:12 PM, Kumar Gala <galak@kernel.crashing.org 
> > wrote:
>>
>> On Feb 17, 2009, at 6:44 AM, <dayu@datangmobile.cn> <dayu@datangmobile.cn 
>> >
>> wrote:
>>
>>> From: Da Yu <dayu@datangmobile.cn>
>>> Date: Tue, 17 Feb 2009 19:58:20 +0800
>>> Subject: [PATCH] fix the interrupt loss problem on powerpc IPIC  
>>> (2.6.23)
>>>
>>> Signed-off-by: Da Yu <dayu@datangmobile.cn>
>>> ---
>>
>> Please provide a bit more description as to why this fixes the issue.
>
> The pending register is write 1 clear.  If there are more than one
> external interrupts pending at the same time, acking the first
> interrupt will also clear other interrupt pending bits.  That will
> cause loss of interrupt.
>
> - Leo

Thanks.  That should be included in the commit message.

- k
Yang Li - Feb. 17, 2009, 2:55 p.m.
On Tue, Feb 17, 2009 at 10:32 PM, Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> On Tue, Feb 17, 2009 at 08:12:33AM -0600, Kumar Gala wrote:
>>
>> On Feb 17, 2009, at 6:44 AM, <dayu@datangmobile.cn> <dayu@datangmobile.cn
>> > wrote:
>>
>>> From: Da Yu <dayu@datangmobile.cn>
>>> Date: Tue, 17 Feb 2009 19:58:20 +0800
>>> Subject: [PATCH] fix the interrupt loss problem on powerpc IPIC
>>> (2.6.23)
>>>
>>> Signed-off-by: Da Yu <dayu@datangmobile.cn>
>>> ---
>>
>> Please provide a bit more description as to why this fixes the issue.
>
> Including whether it still applies to mainline, since your topic seems
> to indicate it's for 2.6.23.  That is pretty old by now.

The problem still exists in mainline.  But the patch doesn't seem to apply.

Dayu,

Could you re-spin the patch for the latest kernel?  Thanks.

- Leo

Patch

--- a/arch/powerpc/sysdev/ipic.c	2009-02-17 15:10:18.000000000 +0800
+++ b/arch/powerpc/sysdev/ipic.c	2009-02-17 20:05:28.000000000 +0800
@@ -561,8 +561,7 @@  static void ipic_ack_irq(unsigned int vi

 	spin_lock_irqsave(&ipic_lock, flags);

-	temp = ipic_read(ipic->regs, ipic_info[src].pend);
-	temp |= (1 << (31 - ipic_info[src].bit));
+	temp = 1 << (31 - ipic_info[src].bit);
 	ipic_write(ipic->regs, ipic_info[src].pend, temp);

 	spin_unlock_irqrestore(&ipic_lock, flags);
@@ -581,8 +580,7 @@  static void ipic_mask_irq_and_ack(unsign
 	temp &= ~(1 << (31 - ipic_info[src].bit));
 	ipic_write(ipic->regs, ipic_info[src].mask, temp);

-	temp = ipic_read(ipic->regs, ipic_info[src].pend);
-	temp |= (1 << (31 - ipic_info[src].bit));
+	temp = 1 << (31 - ipic_info[src].bit);
 	ipic_write(ipic->regs, ipic_info[src].pend, temp);

 	spin_unlock_irqrestore(&ipic_lock, flags);






 

-----邮件原件-----
发件人: Li Yang-R58472 [mailto:LeoLi@freescale.com] 
发送时间: 2009年2月17日 18:17
收件人: 笪禹; linux-kernel@vger.kernel.org
抄送: linuxppc-dev@ozlabs.org
主题: RE: PROBLEM: incorrect interrupt ack lead to interrupt loss on freescale powerpc

> -----Original Message-----
> From: dayu@datangmobile.cn [mailto:dayu@datangmobile.cn]
> Sent: Tuesday, February 17, 2009 4:34 PM
> To: linux-kernel@vger.kernel.org
> Cc: Li Yang-R58472
> Subject: PROBLEM: incorrect interrupt ack lead to interrupt loss on 
> freescale powerpc
> 
>  
> [1.] One line summary of the problem: incorrect interrupt ack  lead to 
> interrupt loss

Acked-by: Li Yang <leoli@freescale.com>

However, please resend the patch with a brief description and Signed-off-by at the top of the patch.  You can read the Documentation/SubmittiongPatches for more information, or even a Chinese version under Documentation/zh_CN/.

Here are some small comments about the patch itself,

--- a/arch/powerpc/sysdev/ipic.c	2009-02-17 15:10:18.000000000 +0800
+++ b/arch/powerpc/sysdev/ipic.c	2009-02-17 15:10:24.000000000 +0800
@@ -9,6 +9,7 @@ 
  * under  the terms of  the GNU General  Public License as published by the
  * Free Software Foundation;  either version 2 of the  License, or (at your
  * option) any later version.
+ * Da Yu <dayu@datangmobile.cn> fixed the interrupt loss problem on 
+ powerpc IPIC


It's not recommended to add changelog in the source now.  Please describe in the patch description area.