On Thu, 21 Feb 2019 at 11:31, Peter Maydell <peter.mayd...@linaro.org> wrote: > > On Fri, 15 Feb 2019 at 15:59, Mark <aln...@airmail.cc> 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. thanks -- PMM
