loongson: flush irq write operation
PATCH 26/30 loongson: flush irq write operation
From d0431e4d83ec7f852d631f80aaca4b66586b4db2 Mon Sep 17 00:00:00 2001
From: Wu Zhangjin <[email protected]>
Date: Sat, 16 May 2009 04:54:27 +0800
Subject: [PATCH 26/30] loongson: flush irq write operation
read back after write, otherwise, there will be many spurious irqs from it
--- arch/mips/kernel/i8259.c | 5 +++++ arch/mips/loongson/common/bonito-irq.c | 4 ++++ 2 files changed, 9 insertions(+), 0 deletions(-)
arch/mips/kernel/i8259.c +2 -0
@@ -175,12 +175,17 @@ handle_real_irq:if (irq &8 ) {inb (PIC_SLAVE_IMR );/* DUMMY - (do we need this?) */ outb (cached_slave_mask,PIC_SLAVE_IMR ); +inb (PIC_SLAVE_IMR );outb (0x60 +(irq&7 ),PIC_SLAVE_CMD );/* 'Specific EOI' to slave */ +inb (PIC_SLAVE_CMD );outb (0x60 +PIC_CASCADE_IR ,PIC_MASTER_CMD );/* 'Specific EOI' to
master-IRQ2 */
+ inb(PIC_MASTER_CMD);
} else {
inb(PIC_MASTER_IMR); /* DUMMY - (do we need this?) */
outb(cached_master_mask, PIC_MASTER_IMR);
+ inb(PIC_SLAVE_IMR);
outb(0x60+irq, PIC_MASTER_CMD); /* 'Specific EOI to master */
+ inb(PIC_MASTER_CMD);
}
smtc_im_ack_irq(irq); spin_unlock_irqrestore(&i8259A_lock, flags);
(quoted diff) +0 -0
b/arch/mips/loongson/common/bonito-irq.c
index d5a5ae8..cfbeaf5 100644
arch/mips/loongson/common/bonito-irq.c +4 -0
@@ -36,12 +36,16 @@static inline void bonito_irq_enable (unsigned int irq) {LOONGSON_INTENSET = (1 << (irq -LOONGSON_IRQ_BASE )); +/* flush the write operation via a following read operation */ + (void )LOONGSON_INTENSET ;mmiowb (); }static inline void bonito_irq_disable (unsigned int irq) {LOONGSON_INTENCLR = (1 << (irq -LOONGSON_IRQ_BASE )); +/* flush the write operation via a following read operation */ + (void )LOONGSON_INTENCLR ;mmiowb (); }
Signature · 1 line
1.6.2.1
Quoted 20 lines · Wu Zhangjin
read back after write, otherwise, there will be many spurious irqs from it
--- arch/mips/kernel/i8259.c | 5 +++++ arch/mips/loongson/common/bonito-irq.c | 4 ++++ 2 files changed, 9 insertions(+), 0 deletions(-)arch/mips/kernel/i8259.c +2
-0@@ -175,12 +175,17 @@ handle_real_irq:if (irq &8 ) {inb (PIC_SLAVE_IMR );/* DUMMY - (do we need this?) */ outb (cached_slave_mask,PIC_SLAVE_IMR ); +inb (PIC_SLAVE_IMR );outb (0x60 +(irq&7 ),PIC_SLAVE_CMD );/* 'Specific EOI' to slave */ +inb (PIC_SLAVE_CMD );outb (0x60 +PIC_CASCADE_IR ,PIC_MASTER_CMD );/* 'Specific EOI' to master-IRQ2 */
arch/mips/kernel/i8259.c
Quoted 10 lines
arch/mips/kernel/i8259.c +3
-0+inb (PIC_MASTER_CMD ); }else {inb (PIC_MASTER_IMR );/* DUMMY - (do we need this?) */ outb (cached_master_mask,PIC_MASTER_IMR ); +inb (PIC_SLAVE_IMR );outb (0x60 +irq,PIC_MASTER_CMD );/* 'Specific EOI to master */ +inb (PIC_MASTER_CMD ); }smtc_im_ack_irq (irq);spin_unlock_irqrestore (&i8259A_lock, flags);
The semantic of inX() / outX() is defined by the x86 architecture which forbids posting I/O port writes. In short I think this one is papering over a bug in the outX() implementation.
I'm ok with the bonito part of this patch.
Ralf
Quoted 28 lines · 在 2009-05-18一的 17:36 +0100，Ralf Baechle写道：
Quoted 26 lines · Wu Zhangjin
read back after write, otherwise, there will be many spurious irqs from it
--- arch/mips/kernel/i8259.c | 5 +++++ arch/mips/loongson/common/bonito-irq.c | 4 ++++ 2 files changed, 9 insertions(+), 0 deletions(-)arch/mips/kernel/i8259.c +2
-0@@ -175,12 +175,17 @@ handle_real_irq:if (irq &8 ) {inb (PIC_SLAVE_IMR );/* DUMMY - (do we need this?) */ outb (cached_slave_mask,PIC_SLAVE_IMR ); +inb (PIC_SLAVE_IMR );outb (0x60 +(irq&7 ),PIC_SLAVE_CMD );/* 'Specific EOI' to slave */ +inb (PIC_SLAVE_CMD );outb (0x60 +PIC_CASCADE_IR ,PIC_MASTER_CMD );/* 'Specific EOI' to master-IRQ2 */
arch/mips/kernel/i8259.c +2
-0+inb (PIC_MASTER_CMD ); }else {inb (PIC_MASTER_IMR );/* DUMMY - (do we need this?) */ outb (cached_master_mask,PIC_MASTER_IMR ); +inb (PIC_SLAVE_IMR );outb (0x60 +irq,PIC_MASTER_CMD );/* 'Specific EOI to master */
Quoted 8 lines
arch/mips/kernel/i8259.c
Quoted 4 lines
arch/mips/kernel/i8259.c +1
-0+inb (PIC_MASTER_CMD ); }smtc_im_ack_irq (irq);spin_unlock_irqrestore (&i8259A_lock, flags);The semantic of
inX()/outX()is defined by the x86 architecture which forbids posting I/O port writes. In short I think this one is papering over a bug in theoutX()implementation.
Yes, the outX should do a delayed write, however it does not. So our solution is making a read to flush the write.
Quoted 4 lines
I'm ok with the bonito part of this patch.
Ralf
Quoted 5 lines · yanh
Quoted 3 lines
The semantic of
inX()/outX()is defined by the x86 architecture which forbids posting I/O port writes. In short I think this one is papering over a bug in theoutX()implementation.Yes, the outX should do a delayed write, however it does not. So our solution is making a read to flush the write.
Do you actually need all the inb() you added to get things to work or is
arch/mips/kernel/i8259.c +2 -0
@@ -177,10 +177,12 @@ handle_real_irq:outb (cached_slave_mask,PIC_SLAVE_IMR );outb (0x60 +(irq&7 ),PIC_SLAVE_CMD );/* 'Specific EOI' to slave */ outb (0x60 +PIC_CASCADE_IR ,PIC_MASTER_CMD );/* 'Specific EOI' to master-IRQ2 */ +inb (PIC_MASTER_CMD ); }else {inb (PIC_MASTER_IMR );/* DUMMY - (do we need this?) */ outb (cached_master_mask,PIC_MASTER_IMR );outb (0x60 +irq,PIC_MASTER_CMD );/* 'Specific EOI to master */ +inb (PIC_MASTER_CMD ); }smtc_im_ack_irq (irq);spin_unlock_irqrestore (&i8259A_lock, flags);
sufficient to solve the problem?
Ralf
Quoted 9 lines · 在 2009-05-19二的 17:01 +0100，Ralf Baechle写道：
Quoted 5 lines · yanh
Quoted 3 lines
The semantic of
inX()/outX()is defined by the x86 architecture which forbids posting I/O port writes. In short I think this one is papering over a bug in theoutX()implementation.Yes, the outX should do a delayed write, however it does not. So our solution is making a read to flush the write.
Do you actually need all the
inb()you added to get things to work or is
Thanks for your reply.
As my test, if there is no the read, there will be many spurious irqs.
Quoted 19 lines
arch/mips/kernel/i8259.c +2
-0@@ -177,10 +177,12 @@ handle_real_irq:outb (cached_slave_mask,PIC_SLAVE_IMR );outb (0x60 +(irq&7 ),PIC_SLAVE_CMD );/* 'Specific EOI' to slave */ outb (0x60 +PIC_CASCADE_IR ,PIC_MASTER_CMD );/* 'Specific EOI' to master-IRQ2 */ +inb (PIC_MASTER_CMD ); }else {inb (PIC_MASTER_IMR );/* DUMMY - (do we need this?) */ outb (cached_master_mask,PIC_MASTER_IMR );outb (0x60 +irq,PIC_MASTER_CMD );/* 'Specific EOI to master */ +inb (PIC_MASTER_CMD ); }smtc_im_ack_irq (irq);spin_unlock_irqrestore (&i8259A_lock, flags);
Quoted 1 line
sufficient to solve the problem?
I have test this patch just now. It works well on yeeloong. I have one question what's the difference between the two patch?
Quoted 2 lines
Ralf
Quoted 32 lines · 在 2009-05-20三的 10:28 +0800，yanh写道：
Quoted 9 lines · 在 2009-05-19二的 17:01 +0100，Ralf Baechle写道：
Quoted 5 lines · yanh
Quoted 3 lines
The semantic of
inX()/outX()is defined by the x86 architecture which forbids posting I/O port writes. In short I think this one is papering over a bug in theoutX()implementation.Yes, the outX should do a delayed write, however it does not. So our solution is making a read to flush the write.
Do you actually need all the
inb()you added to get things to work or isThanks for your reply.
As my test, if there is no the read, there will be many spurious irqs.Quoted 19 lines
arch/mips/kernel/i8259.c +2
-0@@ -177,10 +177,12 @@ handle_real_irq:outb (cached_slave_mask,PIC_SLAVE_IMR );outb (0x60 +(irq&7 ),PIC_SLAVE_CMD );/* 'Specific EOI' to slave */ outb (0x60 +PIC_CASCADE_IR ,PIC_MASTER_CMD );/* 'Specific EOI' to master-IRQ2 */ +inb (PIC_MASTER_CMD ); }else {inb (PIC_MASTER_IMR );/* DUMMY - (do we need this?) */ outb (cached_master_mask,PIC_MASTER_IMR );outb (0x60 +irq,PIC_MASTER_CMD );/* 'Specific EOI to master */ +inb (PIC_MASTER_CMD ); }smtc_im_ack_irq (irq);spin_unlock_irqrestore (&i8259A_lock, flags);
Quoted 3 lines
Quoted 1 line
sufficient to solve the problem?
I have test this patch just now. It works well on yeeloong. I have one question what's the difference between the two patch?
My original patch only flush imr write. Really only one read is suffient. So no question about it now.
Quoted 4 lines
Quoted 2 lines
Ralf
Quoted 1 line · yanh
I have test this patch just now. It works well on yeeloong.
Thanks for testing.
Quoted 1 line
I have one question what's the difference between the two patch?
Uncached writes can't be re-ordered. By adding a read after the last write my patch forces not only completion of the preceding write but due to this ordering constraint also completion of all preceding writes is enforced.
I/O space writes are slow. I mean slower than slugs in space. So my patch is an optimization but that was not the point; I really only meant to verify that we understood what's going on and we seem to. Now let's fix the real issue and make outX() non-posted.
Ralf
Quoted 13 lines · 在 2009-05-20三的 08:07 +0100，Ralf Baechle写道：
Quoted 1 line · yanh
I have test this patch just now. It works well on yeeloong.
Thanks for testing.
Quoted 1 line
I have one question what's the difference between the two patch?
Uncached writes can't be re-ordered. By adding a read after the last write my patch forces not only completion of the preceding write but due to this ordering constraint also completion of all preceding writes is enforced.
Quoted 4 lines
I/O space writes are slow. I mean slower than slugs in space. So my patch is an optimization but that was not the point; I really only meant to verify that we understood what's going on and we seem to. Now let's fix the real issue and make
outX()non-posted.
Thanks for your explanation.
Quoted 3 lines
Ralf