| Message ID | 20190212105201.13795-1-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/arm/armsse: Fix miswiring of expansion IRQs | expand |
On 2/12/19 11:52 AM, Peter Maydell wrote: > In commit 91c1e9fcbd7548db368 where we added dual-CPU support to > the ARMSSE, we set up the wiring of the expansion IRQs via nested > loops: the outer loop on 'i' loops for each CPU, and the inner loop > on 'j' loops for each interrupt. Fix a typo which meant we were > wiring every expansion IRQ line to external IRQ 0 on CPU 0 and > to external IRQ 1 on CPU 1. > > Fixes: 91c1e9fcbd7548db368 ("hw/arm/armsse: Support dual-CPU configuration") > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > It turns out that the ARM-TFM image I was using to test that > I hadn't broken the mps2-an505 doesn't actually rely on any > interrupts from the external devices... How did you notice that, simply reviewing? Via 'info qtree'? > > hw/arm/armsse.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c > index 5d53071a5a0..9a8c49547db 100644 > --- a/hw/arm/armsse.c > +++ b/hw/arm/armsse.c > @@ -565,7 +565,7 @@ static void armsse_realize(DeviceState *dev, Error **errp) > /* Connect EXP_IRQ/EXP_CPUn_IRQ GPIOs to the NVIC's lines 32 and up */ > s->exp_irqs[i] = g_new(qemu_irq, s->exp_numirq); > for (j = 0; j < s->exp_numirq; j++) { > - s->exp_irqs[i][j] = qdev_get_gpio_in(cpudev, i + 32); > + s->exp_irqs[i][j] = qdev_get_gpio_in(cpudev, j + 32); > } > if (i == 0) { > gpioname = g_strdup("EXP_IRQ"); >
On Tue, 12 Feb 2019 at 11:05, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > On 2/12/19 11:52 AM, Peter Maydell wrote: > > In commit 91c1e9fcbd7548db368 where we added dual-CPU support to > > the ARMSSE, we set up the wiring of the expansion IRQs via nested > > loops: the outer loop on 'i' loops for each CPU, and the inner loop > > on 'j' loops for each interrupt. Fix a typo which meant we were > > wiring every expansion IRQ line to external IRQ 0 on CPU 0 and > > to external IRQ 1 on CPU 1. > > > > Fixes: 91c1e9fcbd7548db368 ("hw/arm/armsse: Support dual-CPU configuration") > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > --- > > It turns out that the ARM-TFM image I was using to test that > > I hadn't broken the mps2-an505 doesn't actually rely on any > > interrupts from the external devices... > > How did you notice that, simply reviewing? Via 'info qtree'? I'm working on a model of a different board (Musca) which also uses the SSE-200, and the test code I had for that does happen to care about the interrupts. thanks -- PMM
On 2/12/19 12:06 PM, Peter Maydell wrote: > On Tue, 12 Feb 2019 at 11:05, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >> On 2/12/19 11:52 AM, Peter Maydell wrote: >>> In commit 91c1e9fcbd7548db368 where we added dual-CPU support to >>> the ARMSSE, we set up the wiring of the expansion IRQs via nested >>> loops: the outer loop on 'i' loops for each CPU, and the inner loop >>> on 'j' loops for each interrupt. Fix a typo which meant we were >>> wiring every expansion IRQ line to external IRQ 0 on CPU 0 and >>> to external IRQ 1 on CPU 1. >>> >>> Fixes: 91c1e9fcbd7548db368 ("hw/arm/armsse: Support dual-CPU configuration") >>> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> >>> --- >>> It turns out that the ARM-TFM image I was using to test that >>> I hadn't broken the mps2-an505 doesn't actually rely on any >>> interrupts from the external devices... >> >> How did you notice that, simply reviewing? Via 'info qtree'? > > I'm working on a model of a different board (Musca) which also > uses the SSE-200, and the test code I had for that does happen > to care about the interrupts. OK, welcome Musca! If you are testing Open Source code, please add a few notes on the wiki or in a mail (or the cover when you introduce the Musca), so we can add an acceptance test. Thanks, Phil.
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index 5d53071a5a0..9a8c49547db 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -565,7 +565,7 @@ static void armsse_realize(DeviceState *dev, Error **errp) /* Connect EXP_IRQ/EXP_CPUn_IRQ GPIOs to the NVIC's lines 32 and up */ s->exp_irqs[i] = g_new(qemu_irq, s->exp_numirq); for (j = 0; j < s->exp_numirq; j++) { - s->exp_irqs[i][j] = qdev_get_gpio_in(cpudev, i + 32); + s->exp_irqs[i][j] = qdev_get_gpio_in(cpudev, j + 32); } if (i == 0) { gpioname = g_strdup("EXP_IRQ");
In commit 91c1e9fcbd7548db368 where we added dual-CPU support to the ARMSSE, we set up the wiring of the expansion IRQs via nested loops: the outer loop on 'i' loops for each CPU, and the inner loop on 'j' loops for each interrupt. Fix a typo which meant we were wiring every expansion IRQ line to external IRQ 0 on CPU 0 and to external IRQ 1 on CPU 1. Fixes: 91c1e9fcbd7548db368 ("hw/arm/armsse: Support dual-CPU configuration") Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- It turns out that the ARM-TFM image I was using to test that I hadn't broken the mps2-an505 doesn't actually rely on any interrupts from the external devices... hw/arm/armsse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)