Skip to contentSkip to conversation

Xilinx Intc Fixes

PATCH Applied [email protected] 2 participants 7 messages Jun 11, 2013 – Jun 18, 2013 Arm/Zynq architecture

lore
Share & tools
lore

[email protected]

author
#0 lore raw

v2 cover Xilinx Intc Fixes

From: Peter Crosthwaite <[email protected]>

Various fixups to the Xilinx Interrupt controller following a review against TRM and RTL descriptions.

Tested as working for microblaze and microblazeel Linux.

change from v1:
Fixed S3ADSP UART interrupt - done first for bisectability (Now tested as working on s3asdsp design)

Peter Crosthwaite (5):
microblaze/petalogix_s3adsp1800_mmu: Fix UART IRQ intc/xilinx_intc: Don't clear level sens. IRQs without ACK intc/xilinx_intc: Handle level interrupt retriggering intc/xilinx_intc: Inhibit write to ISR when HIE intc/xilinx_intc: Dont lower IRQ when HIE cleared

 hw/intc/xilinx_intc.c                    | 28 ++++++++++++++++++----------
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  2 +-
 2 files changed, 19 insertions(+), 11 deletions(-)
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
author
#1 lore raw

PATCH v2 1/5 microblaze/petalogix_s3adsp1800_mmu: Fix UART IRQ

From: Peter Crosthwaite <[email protected]>

The UART IRQ is edge sensitive, whereas the machine was registering it as level sensitive. Fix.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
hw/microblaze/petalogix_s3adsp1800_mmu.c +1 -1
index 7c258f0..b3bcd4e 100644
@@ -97,7 +97,7 @@ petalogix_s3adsp1800_init(QEMUMachineInitArgs *args)
                           1, 0x89, 0x18, 0x0000, 0x0, 1);
 
     cpu_irq = microblaze_pic_init_cpu(env);
-    dev = xilinx_intc_create(INTC_BASEADDR, cpu_irq[0], 2);
+    dev = xilinx_intc_create(INTC_BASEADDR, cpu_irq[0], 0xA);
     for (i = 0; i < 32; i++) {
         irq[i] = qdev_get_gpio_in(dev, i);
     }
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
author
#2 lore raw

PATCH v2 2/5 intc/xilinx_intc: Don't clear level sens. IRQs without ACK

In reply to [email protected]

From: Peter Crosthwaite <[email protected]>

For level sensitive interrupts, ISR bits are cleared when the input pin is lowered. This is incorrect. Only software can clear ISR bits (via IAR or direct write to ISR with !MER(2)).

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/intc/xilinx_intc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
hw/intc/xilinx_intc.c +1 -7
index b106e72..a585ba1 100644
@@ -139,13 +139,7 @@ static void irq_handler(void *opaque, int irq, int level)
         return;
     }
 
-    /* Update source flops. Don't clear unless level triggered.
-       Edge triggered interrupts only go away when explicitely acked to
-       the interrupt controller.  */
-    if (!(p->c_kind_of_intr & (1 << irq)) || level) {
-        p->regs[R_ISR] &= ~(1 << irq);
-        p->regs[R_ISR] |= (level << irq);
-    }
+    p->regs[R_ISR] |= (level << irq);
     update_irq(p);
 }
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
author
#3 lore raw

PATCH v2 3/5 intc/xilinx_intc: Handle level interrupt retriggering

In reply to [email protected]

From: Peter Crosthwaite <[email protected]>

Acking a level sensitive interrupt should have no effect if the interrupt pin is still asserted. The current implementation requires and edge condition to occur for setting a level sensitive IRQ, which means an ACK can clear a level sensitive interrupt, until the original source strobes the interrupt again.

Fix by keeping track of the interrupt pin state and setting ISR based on this every time update_irq() is called.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/intc/xilinx_intc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
hw/intc/xilinx_intc.c +15 -1
index a585ba1..010b080 100644
@@ -49,11 +49,19 @@ struct xlx_pic
 
     /* Runtime control registers.  */
     uint32_t regs[R_MAX];
+    /* state of the interrupt input pins */
+    uint32_t irq_pin_state;
 };
 
 static void update_irq(struct xlx_pic *p)
 {
     uint32_t i;
+
+    /* level triggered interrupt */
+    if (p->regs[R_MER] & 2) {
+        p->regs[R_ISR] |= p->irq_pin_state & ~p->c_kind_of_intr;
+    }
+
     /* Update the pending register.  */
     p->regs[R_IPR] = p->regs[R_ISR] & p->regs[R_IER];
 

@@ -139,7 +147,13 @@ static void irq_handler(void *opaque, int irq, int level)
         return;
     }
 
-    p->regs[R_ISR] |= (level << irq);
+    /* edge triggered interrupt */
+    if (p->c_kind_of_intr & (1 << irq) && p->regs[R_MER] & 2) {
+        p->regs[R_ISR] |= (level << irq);
+    }
+
+    p->irq_pin_state &= ~(1 << irq);
+    p->irq_pin_state |= level << irq;
     update_irq(p);
 }
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
author
#4 lore raw

PATCH v2 4/5 intc/xilinx_intc: Inhibit write to ISR when HIE

In reply to [email protected]

From: Peter Crosthwaite <[email protected]>

When the Hardware Interrupt Enable (HIE) bit is set, software cannot change ISR. Add write guard accordingly.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/intc/xilinx_intc.c | 5 +++++
 1 file changed, 5 insertions(+)
hw/intc/xilinx_intc.c +5 -0
index 010b080..e9121cd 100644
@@ -120,6 +120,11 @@ pic_write(void *opaque, hwaddr addr,
         case R_CIE:
             p->regs[R_IER] &= ~value; /* Atomic clear ie.  */
             break;
+        case R_ISR:
+            if ((p->regs[R_MER] & 2)) {
+                break;
+            }
+            /* fallthrough */
         default:
             if (addr < ARRAY_SIZE(p->regs))
                 p->regs[addr] = value;
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
author
#5 lore raw

PATCH v2 5/5 intc/xilinx_intc: Dont lower IRQ when HIE cleared

In reply to [email protected]

From: Peter Crosthwaite <[email protected]>

This is a little strange. It is lowering the parent IRQ pin on input when HIE is cleared. There is no such behaviour in the real hardware.

ISR changes based on interrupt pin state are already guarded on HIE being set. So we can just delete this if in its entirety.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/intc/xilinx_intc.c | 5 -----
 1 file changed, 5 deletions(-)
hw/intc/xilinx_intc.c +0 -5
index e9121cd..f343ee0 100644
@@ -147,11 +147,6 @@ static void irq_handler(void *opaque, int irq, int level)
 {
     struct xlx_pic *p = opaque;
 
-    if (!(p->regs[R_MER] & 2)) {
-        qemu_irq_lower(p->parent_irq);
-        return;
-    }
-
     /* edge triggered interrupt */
     if (p->c_kind_of_intr & (1 << irq) && p->regs[R_MER] & 2) {
         p->regs[R_ISR] |= (level << irq);
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
Applied
#6 lore raw
In reply to [email protected]
Quoted 11 lines · On Tue, Jun 11, 2013 at 10:56:55AM +1000, [email protected] wrote:

From: Peter Crosthwaite <[email protected]>

Various fixups to the Xilinx Interrupt controller following a review against TRM and RTL descriptions.

Tested as working for microblaze and microblazeel Linux.

change from v1:
Fixed S3ADSP UART interrupt - done first for bisectability (Now tested as working on s3asdsp design)

Applied, thanks Peter.

Cheers,
Edgar

Quoted 16 lines

Peter Crosthwaite (5):
microblaze/petalogix_s3adsp1800_mmu: Fix UART IRQ intc/xilinx_intc: Don't clear level sens. IRQs without ACK intc/xilinx_intc: Handle level interrupt retriggering intc/xilinx_intc: Inhibit write to ISR when HIE intc/xilinx_intc: Dont lower IRQ when HIE cleared

 hw/intc/xilinx_intc.c                    | 28 ++++++++++++++++++----------
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  2 +-
 2 files changed, 19 insertions(+), 11 deletions(-)
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty