Re: [Qemu-devel] [PATCH v1 2/4] hw/riscv/sifive_plic: Use gpios instead of irqs

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

[-- Attachment #1: Type: text/plain, Size: 5403 bytes --]

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.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]