[Top][All Lists]

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

Re: [Qemu-devel] [v4 PATCH] hw/arm/bcm2835_peripherals: add bcm283x arm

From: Peter Maydell
Subject: Re: [Qemu-devel] [v4 PATCH] hw/arm/bcm2835_peripherals: add bcm283x arm timer
Date: Tue, 26 Feb 2019 14:31:16 +0000

On Thu, 21 Feb 2019 at 11:31, Peter Maydell <address@hidden> wrote:
> On Fri, 15 Feb 2019 at 15:59, Mark <address@hidden> wrote:
> > +    s->irq = qemu_allocate_irq(bcm283x_timer_set_irq, s, 0);
> I know this code has come from the sp804 code, but it's
> actually wrong (and I should fix the sp804). qemu_allocate_irq()
> is for interrupt lines that come into a device, not for ones
> that go out from it, like this one. The call happens to have
> no visible bad effects because what it does will be overrriden
> later when the board code wires up the interrupt to the
> interrupt controller. (There will be a tiny memory leak.)

Looking more closely at this, my analysis here is wrong -- sorry.
For the sp804, this call is necessary, because the sp804
has a dual layer structure between the arm_timer_state
layer and the SP804State layer. It creates outbound IRQs
from the arm_timer_state with qemu_allocate_irq(), and
effectively connects them to the SP804State via the
sp804_set_irq() function (which raises the outbound IRQ from
the SP804State if either of the arm_timer_state lines is high).

In your case there is no dual-layer structure, so you don't
need to call qemu_allocate_irq() at all. (You're effectively
trying to use BCM283xTimerState as both the inner layer IRQ
and the outer layer IRQ, and it happens to work because the
setup of the outer layer happens last so the bogus inner
layer initialization is ignored.)

> You can just delete the qemu_allocate_irq() call here, and
> the bcm283x_timer_set_irq() function.

So my recommendation here was correct, but my reasoning was
not right.

-- PMM

reply via email to

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