[Qemu-devel] [PATCH v2 0/5] Xilinx Intc Fixes

[Qemu-devel] [PATCH v2 0/5] Xilinx Intc Fixes

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>


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(-)

-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH v2 1/5] microblaze/petalogix_s3adsp1800_mmu: Fix UART IRQ

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 7c258f0..b3bcd4e 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -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);
     }
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH v2 2/5] intc/xilinx_intc: Don't clear level sens. IRQs without ACK

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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 <peter.crosthwaite@xilinx.com>
---

 hw/intc/xilinx_intc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index b106e72..a585ba1 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -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);
 }
 
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH v2 3/5] intc/xilinx_intc: Handle level interrupt retriggering

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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 <peter.crosthwaite@xilinx.com>
---

 hw/intc/xilinx_intc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index a585ba1..010b080 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -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);
 }
 
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH v2 4/5] intc/xilinx_intc: Inhibit write to ISR when HIE

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/intc/xilinx_intc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index 010b080..e9121cd 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -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;
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH v2 5/5] intc/xilinx_intc: Dont lower IRQ when HIE cleared

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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 <peter.crosthwaite@xilinx.com>
---

 hw/intc/xilinx_intc.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index e9121cd..f343ee0 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -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);
-- 
1.8.3.rc1.44.gb387c77.dirty

Re: [Qemu-devel] [PATCH v2 0/5] Xilinx Intc Fixes

[Edgar E. Iglesias]

On Tue, Jun 11, 2013 at 10:56:55AM +1000, peter.crosthwaite@xilinx.com wrote:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> 
> 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


> 
> 
> 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(-)
> 
> -- 
> 1.8.3.rc1.44.gb387c77.dirty
>