qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/7] allwinner-a10-pic: fix interrupt clear beha


From: Li Guang
Subject: Re: [Qemu-devel] [PATCH 2/7] allwinner-a10-pic: fix interrupt clear behaviour
Date: Tue, 25 Feb 2014 09:08:24 +0800
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1.16) Gecko/20120421 Iceape/2.0.11

Beniamino Galvani wrote:
On Mon, Feb 24, 2014 at 02:45:06PM +0800, Li Guang wrote:
Beniamino Galvani wrote:
On Wed, Feb 19, 2014 at 10:02:36AM +0800, Li Guang wrote:
Beniamino Galvani wrote:
On Tue, Feb 18, 2014 at 11:49:51AM +0800, Li Guang wrote:
Beniamino Galvani wrote:
According to this mail thread [1], writing to pending register seems
to have no effect on actual pending status of interrupts. This means
that the only way to clear a pending interrupt is to clear the
interrupt source. This patch implements such behaviour.

[1] http://lkml.org/lkml/2013/7/6/59

Signed-off-by: Beniamino Galvani<address@hidden>
---
  hw/intc/allwinner-a10-pic.c |    6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/intc/allwinner-a10-pic.c b/hw/intc/allwinner-a10-pic.c
index bb2351f..afd57ef 100644
--- a/hw/intc/allwinner-a10-pic.c
+++ b/hw/intc/allwinner-a10-pic.c
@@ -49,6 +49,8 @@ static void aw_a10_pic_set_irq(void *opaque, int irq, int 
level)

      if (level) {
          set_bit(irq % 32, (void *)&s->irq_pending[irq / 32]);
+    } else {
+        clear_bit(irq % 32, (void *)&s->irq_pending[irq / 32]);
      }
      aw_a10_pic_update(s);
  }
@@ -105,10 +107,10 @@ static void aw_a10_pic_write(void *opaque, hwaddr offset, 
uint64_t value,
          s->nmi = value;
          break;
      case AW_A10_PIC_IRQ_PENDING ... AW_A10_PIC_IRQ_PENDING + 8:
-        s->irq_pending[index]&= ~value;
+        /* Nothing to do */
          break;
      case AW_A10_PIC_FIQ_PENDING ... AW_A10_PIC_FIQ_PENDING + 8:
-        s->fiq_pending[index]&= ~value;
+        /* Ditto */
          break;
      case AW_A10_PIC_SELECT ... AW_A10_PIC_SELECT + 8:
          s->select[index] = value;
pending registers are also clear registers by a10 datasheet,
also you found bits are marked as 'R', so, ..., contradict itself.
Yes, the datasheet is inconsistent about this because the register
can't be read-only and 'clear' at the same time.

Unfortunately at the moment I cannot test if the clearing
functionality of the pending register works on real hardware but the
idea I got from the linked discussion is that it's either not
implemented or broken and therefore interrupts remain pending until
they are disabled at the source.

Do you have a chance to try it on a real board?

Ah? even kernel code from allwinner wrote pending registers
to clear pending interrupt, didn't you see it?
so should be no doubt that these registers are writable.
Well, if you look closely at that code, it's a bit strange:

void sw_irq_ack(struct irq_data *irqd)
{
     unsigned int irq = irqd->irq;

     [...]
     writel(readl(SW_INT_IRQ_PENDING_REG0) | (1<<irq), SW_INT_IRQ_PENDING_REG0);
     [...]
}

In order to clear a single interrupt, it is or'ing the irq bit with
the previous value of the register, so it's basically clearing all
pending interrupts.

This is discussed in the mentioned thread from lkml and the final
explanation is that the pending register is always updated with the
actual status of irq lines, even if you write to it. In other words,
writes to the register are ignored, otherwise the code above would
produce random loss of interrupts.

Hmm..., sorry,  I also can't test this operation on A10 board now,
but why not they just wipe out these writings(kernel 3.12)?
I don't know, there was a proposed patch that removed those writes in
the lkml discussion but probably it never reached mainline.

To be on the safe side I can restore the writability of the register
and then, when we will figure out how it really works, correct it if
needed.



Agreed,

Thanks!





reply via email to

[Prev in Thread] Current Thread [Next in Thread]