On Wed, May 9, 2018 at 7:33 PM, Philippe Mathieu-Daudé <f4...@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.fran...@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.
>
