| Message ID | 6634a98e5558a060369ac365e8ea2fd960af5b81.1525464177.git.alistair.francis@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: SoCify the SiFive boards and connect the | expand |
Hi Alistair, On 05/04/2018 05:13 PM, Alistair Francis wrote: > Instead of creating the interrupt in lines with qemu_allocate_irq() use > qdev_init_gpio_in() as this gives us the ability to use the qdev*gpio*() > helpers later on. This is a good idea, but your patch is incomplete: The devices previously using plic->irqs[] now need to use those qdev*gpio*() helpers. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/riscv/sifive_plic.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c > index a4ac910ca9..81b6b5245b 100644 > --- a/hw/riscv/sifive_plic.c > +++ b/hw/riscv/sifive_plic.c > @@ -431,7 +431,6 @@ static void sifive_plic_irq_request(void *opaque, int irq, int level) > static void sifive_plic_realize(DeviceState *dev, Error **errp) > { > SiFivePLICState *plic = SIFIVE_PLIC(dev); > - int i; > > memory_region_init_io(&plic->mmio, OBJECT(dev), &sifive_plic_ops, plic, > TYPE_SIFIVE_PLIC, plic->aperture_size); > @@ -444,9 +443,7 @@ static void sifive_plic_realize(DeviceState *dev, Error **errp) > plic->enable = g_new0(uint32_t, plic->bitfield_words * plic->num_addrs); > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &plic->mmio); > plic->irqs = g_new0(qemu_irq, plic->num_sources + 1); > - for (i = 0; i <= plic->num_sources; i++) { > - plic->irqs[i] = qemu_allocate_irq(sifive_plic_irq_request, plic, i); > - } > + qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources); > } > > static void sifive_plic_class_init(ObjectClass *klass, void *data) > The following patch completes yours: - access the irqs with qdev_get_gpio_in() - remove plic->irqs[] alloc in sifive_plic_realize() - remove plic->irqs[] from SiFivePLICState struct -- >8 -- include/hw/riscv/sifive_plic.h | 1 - hw/riscv/sifive_e.c | 2 +- hw/riscv/sifive_plic.c | 1 - hw/riscv/sifive_u.c | 2 +- hw/riscv/virt.c | 4 ++-- 5 files changed, 4 insertions(+), 6 deletions(-) diff --git a/include/hw/riscv/sifive_plic.h b/include/hw/riscv/sifive_plic.h index 11a5a98df1..2f2af7e686 100644 --- a/include/hw/riscv/sifive_plic.h +++ b/include/hw/riscv/sifive_plic.h @@ -56,7 +56,6 @@ typedef struct SiFivePLICState { uint32_t *claimed; uint32_t *enable; QemuMutex lock; - qemu_irq *irqs; /* config */ char *hart_config; diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index 0ab5e3ca45..7b4f04adcf 100644 --- a/hw/riscv/sifive_e.c +++ b/hw/riscv/sifive_e.c @@ -190,7 +190,7 @@ static void riscv_sifive_e31_realize(DeviceState *dev, Error **errp) sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0", memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size); sifive_uart_create(sys_mem, memmap[SIFIVE_E_UART0].base, - serial_hd(0), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_E_UART0_IRQ]); + serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_E_UART0_IRQ)); sifive_mmio_emulate(sys_mem, "riscv.sifive.e.qspi0", memmap[SIFIVE_E_QSPI0].base, memmap[SIFIVE_E_QSPI0].size); sifive_mmio_emulate(sys_mem, "riscv.sifive.e.pwm0", diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c index c6ad17057d..a91aeb97ab 100644 --- a/hw/riscv/sifive_plic.c +++ b/hw/riscv/sifive_plic.c @@ -447,7 +447,6 @@ static void sifive_plic_realize(DeviceState *dev, Error **errp) plic->claimed = g_new0(uint32_t, plic->bitfield_words); plic->enable = g_new0(uint32_t, plic->bitfield_words * plic->num_addrs); sysbus_init_mmio(SYS_BUS_DEVICE(dev), &plic->mmio); - plic->irqs = g_new0(qemu_irq, plic->num_sources + 1); qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources); } diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index 54f33c39ae..b7936dcec5 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -330,7 +330,7 @@ static void riscv_sifive_u54_realize(DeviceState *dev, Error **errp) SIFIVE_U_PLIC_CONTEXT_STRIDE, memmap[SIFIVE_U_PLIC].size); sifive_uart_create(system_memory, memmap[SIFIVE_U_UART0].base, - serial_hd(0), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART0_IRQ]); + serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_U_UART0_IRQ)); /* sifive_uart_create(system_memory, memmap[SIFIVE_U_UART1].base, serial_hd(1), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART1_IRQ]); */ sifive_clint_create(memmap[SIFIVE_U_CLINT].base, diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index ad03113e0f..bdd75722eb 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -379,11 +379,11 @@ static void riscv_virt_board_init(MachineState *machine) for (i = 0; i < VIRTIO_COUNT; i++) { sysbus_create_simple("virtio-mmio", memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size, - SIFIVE_PLIC(s->plic)->irqs[VIRTIO_IRQ + i]); + qdev_get_gpio_in(DEVICE(s->plic), VIRTIO_IRQ + i)); } serial_mm_init(system_memory, memmap[VIRT_UART0].base, - 0, SIFIVE_PLIC(s->plic)->irqs[UART0_IRQ], 399193, + 0, qdev_get_gpio_in(DEVICE(s->plic), UART0_IRQ), 399193, serial_hd(0), DEVICE_LITTLE_ENDIAN); } -- Regards, Phil.
On Wed, May 9, 2018 at 7:33 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Hi Alistair, > > On 05/04/2018 05:13 PM, Alistair Francis wrote: >> Instead of creating the interrupt in lines with qemu_allocate_irq() use >> qdev_init_gpio_in() as this gives us the ability to use the qdev*gpio*() >> helpers later on. > > This is a good idea, but your patch is incomplete: > The devices previously using plic->irqs[] now need to use those > qdev*gpio*() helpers. > >> >> Signed-off-by: Alistair Francis <alistair.francis@wdc.com> >> --- >> hw/riscv/sifive_plic.c | 5 +---- >> 1 file changed, 1 insertion(+), 4 deletions(-) >> >> diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c >> index a4ac910ca9..81b6b5245b 100644 >> --- a/hw/riscv/sifive_plic.c >> +++ b/hw/riscv/sifive_plic.c >> @@ -431,7 +431,6 @@ static void sifive_plic_irq_request(void *opaque, int irq, int level) >> static void sifive_plic_realize(DeviceState *dev, Error **errp) >> { >> SiFivePLICState *plic = SIFIVE_PLIC(dev); >> - int i; >> >> memory_region_init_io(&plic->mmio, OBJECT(dev), &sifive_plic_ops, plic, >> TYPE_SIFIVE_PLIC, plic->aperture_size); >> @@ -444,9 +443,7 @@ static void sifive_plic_realize(DeviceState *dev, Error **errp) >> plic->enable = g_new0(uint32_t, plic->bitfield_words * plic->num_addrs); >> sysbus_init_mmio(SYS_BUS_DEVICE(dev), &plic->mmio); >> plic->irqs = g_new0(qemu_irq, plic->num_sources + 1); >> - for (i = 0; i <= plic->num_sources; i++) { >> - plic->irqs[i] = qemu_allocate_irq(sifive_plic_irq_request, plic, i); >> - } >> + qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources); >> } >> >> static void sifive_plic_class_init(ObjectClass *klass, void *data) >> > > The following patch completes yours: > > - access the irqs with qdev_get_gpio_in() > - remove plic->irqs[] alloc in sifive_plic_realize() > - remove plic->irqs[] from SiFivePLICState struct Thanks for that. I realised the same thing. This series is also missing some device tree changes that are required. I'll fix all of that in the next version. Alistair > > -- >8 -- > include/hw/riscv/sifive_plic.h | 1 - > hw/riscv/sifive_e.c | 2 +- > hw/riscv/sifive_plic.c | 1 - > hw/riscv/sifive_u.c | 2 +- > hw/riscv/virt.c | 4 ++-- > 5 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/include/hw/riscv/sifive_plic.h b/include/hw/riscv/sifive_plic.h > index 11a5a98df1..2f2af7e686 100644 > --- a/include/hw/riscv/sifive_plic.h > +++ b/include/hw/riscv/sifive_plic.h > @@ -56,7 +56,6 @@ typedef struct SiFivePLICState { > uint32_t *claimed; > uint32_t *enable; > QemuMutex lock; > - qemu_irq *irqs; > > /* config */ > char *hart_config; > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c > index 0ab5e3ca45..7b4f04adcf 100644 > --- a/hw/riscv/sifive_e.c > +++ b/hw/riscv/sifive_e.c > @@ -190,7 +190,7 @@ static void riscv_sifive_e31_realize(DeviceState > *dev, Error **errp) > sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0", > memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size); > sifive_uart_create(sys_mem, memmap[SIFIVE_E_UART0].base, > - serial_hd(0), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_E_UART0_IRQ]); > + serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic), > SIFIVE_E_UART0_IRQ)); > sifive_mmio_emulate(sys_mem, "riscv.sifive.e.qspi0", > memmap[SIFIVE_E_QSPI0].base, memmap[SIFIVE_E_QSPI0].size); > sifive_mmio_emulate(sys_mem, "riscv.sifive.e.pwm0", > diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c > index c6ad17057d..a91aeb97ab 100644 > --- a/hw/riscv/sifive_plic.c > +++ b/hw/riscv/sifive_plic.c > @@ -447,7 +447,6 @@ static void sifive_plic_realize(DeviceState *dev, > Error **errp) > plic->claimed = g_new0(uint32_t, plic->bitfield_words); > plic->enable = g_new0(uint32_t, plic->bitfield_words * > plic->num_addrs); > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &plic->mmio); > - plic->irqs = g_new0(qemu_irq, plic->num_sources + 1); > qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources); > } > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > index 54f33c39ae..b7936dcec5 100644 > --- a/hw/riscv/sifive_u.c > +++ b/hw/riscv/sifive_u.c > @@ -330,7 +330,7 @@ static void riscv_sifive_u54_realize(DeviceState > *dev, Error **errp) > SIFIVE_U_PLIC_CONTEXT_STRIDE, > memmap[SIFIVE_U_PLIC].size); > sifive_uart_create(system_memory, memmap[SIFIVE_U_UART0].base, > - serial_hd(0), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART0_IRQ]); > + serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic), > SIFIVE_U_UART0_IRQ)); > /* sifive_uart_create(system_memory, memmap[SIFIVE_U_UART1].base, > serial_hd(1), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART1_IRQ]); */ > sifive_clint_create(memmap[SIFIVE_U_CLINT].base, > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index ad03113e0f..bdd75722eb 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -379,11 +379,11 @@ static void riscv_virt_board_init(MachineState > *machine) > for (i = 0; i < VIRTIO_COUNT; i++) { > sysbus_create_simple("virtio-mmio", > memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size, > - SIFIVE_PLIC(s->plic)->irqs[VIRTIO_IRQ + i]); > + qdev_get_gpio_in(DEVICE(s->plic), VIRTIO_IRQ + i)); > } > > serial_mm_init(system_memory, memmap[VIRT_UART0].base, > - 0, SIFIVE_PLIC(s->plic)->irqs[UART0_IRQ], 399193, > + 0, qdev_get_gpio_in(DEVICE(s->plic), UART0_IRQ), 399193, > serial_hd(0), DEVICE_LITTLE_ENDIAN); > } > > -- > > Regards, > > Phil. >
diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c index a4ac910ca9..81b6b5245b 100644 --- a/hw/riscv/sifive_plic.c +++ b/hw/riscv/sifive_plic.c @@ -431,7 +431,6 @@ static void sifive_plic_irq_request(void *opaque, int irq, int level) static void sifive_plic_realize(DeviceState *dev, Error **errp) { SiFivePLICState *plic = SIFIVE_PLIC(dev); - int i; memory_region_init_io(&plic->mmio, OBJECT(dev), &sifive_plic_ops, plic, TYPE_SIFIVE_PLIC, plic->aperture_size); @@ -444,9 +443,7 @@ static void sifive_plic_realize(DeviceState *dev, Error **errp) plic->enable = g_new0(uint32_t, plic->bitfield_words * plic->num_addrs); sysbus_init_mmio(SYS_BUS_DEVICE(dev), &plic->mmio); plic->irqs = g_new0(qemu_irq, plic->num_sources + 1); - for (i = 0; i <= plic->num_sources; i++) { - plic->irqs[i] = qemu_allocate_irq(sifive_plic_irq_request, plic, i); - } + qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources); } static void sifive_plic_class_init(ObjectClass *klass, void *data)
Instead of creating the interrupt in lines with qemu_allocate_irq() use qdev_init_gpio_in() as this gives us the ability to use the qdev*gpio*() helpers later on. Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- hw/riscv/sifive_plic.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)