[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: |
Beniamino Galvani |
Subject: |
Re: [Qemu-devel] [PATCH 2/7] allwinner-a10-pic: fix interrupt clear behaviour |
Date: |
Mon, 24 Feb 2014 23:50:04 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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.
Beniamino
- [Qemu-devel] [PATCH 0/7] Allwinner A10 fixes, Beniamino Galvani, 2014/02/17
- [Qemu-devel] [PATCH 5/7] allwinner-a10-pit: implement prescaler and source selection, Beniamino Galvani, 2014/02/17
- [Qemu-devel] [PATCH 2/7] allwinner-a10-pic: fix interrupt clear behaviour, Beniamino Galvani, 2014/02/17
- Re: [Qemu-devel] [PATCH 2/7] allwinner-a10-pic: fix interrupt clear behaviour, Li Guang, 2014/02/17
- Re: [Qemu-devel] [PATCH 2/7] allwinner-a10-pic: fix interrupt clear behaviour, Beniamino Galvani, 2014/02/18
- Re: [Qemu-devel] [PATCH 2/7] allwinner-a10-pic: fix interrupt clear behaviour, Li Guang, 2014/02/18
- Re: [Qemu-devel] [PATCH 2/7] allwinner-a10-pic: fix interrupt clear behaviour, Beniamino Galvani, 2014/02/22
- Re: [Qemu-devel] [PATCH 2/7] allwinner-a10-pic: fix interrupt clear behaviour, Li Guang, 2014/02/24
- Re: [Qemu-devel] [PATCH 2/7] allwinner-a10-pic: fix interrupt clear behaviour,
Beniamino Galvani <=
- Re: [Qemu-devel] [PATCH 2/7] allwinner-a10-pic: fix interrupt clear behaviour, Li Guang, 2014/02/24
[Qemu-devel] [PATCH 7/7] allwinner-emac: update irq status after writes to interrupt registers, Beniamino Galvani, 2014/02/17
[Qemu-devel] [PATCH 4/7] allwinner-a10-pit: use level triggered interrupts, Beniamino Galvani, 2014/02/17
[Qemu-devel] [PATCH 3/7] allwinner-a10-pit: avoid generation of spurious interrupts, Beniamino Galvani, 2014/02/17
[Qemu-devel] [PATCH 1/7] allwinner-a10-pic: set vector address when an interrupt is pending, Beniamino Galvani, 2014/02/17