Re: [PATCH v4 12/16] aspeed/soc: Add AST2700 support

[Cédric Le Goater]

Hello Jamin,

I refer to versal_create_apu_gic function, 
https://github.com/qemu/qemu/blob/master/hw/arm/xlnx-versal.c#L67
and updated aspeed_soc_ast2700_gic as following.
If you have any concerned about the new changes, please let me know.
Thanks-Jamin

static bool aspeed_soc_ast2700_gic(DeviceState *dev, Error **errp)

Please rename to aspeed_soc_ast2700_gic_realize()

{
     Aspeed27x0SoCState *a = ASPEED27X0_SOC(dev);
     AspeedSoCState *s = ASPEED_SOC(dev);
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
     SysBusDevice *gicbusdev;
     DeviceState *gicdev;
     QList *redist_region_count;
     int i;

     object_initialize_child(OBJECT(a), "ast2700-gic", &a->gic,
                             gicv3_class_name());

and object_initialize_child() can be called in aspeed_soc_ast2700_init().

     gicbusdev = SYS_BUS_DEVICE(&a->gic);
     gicdev = DEVICE(&a->gic);
     qdev_prop_set_uint32(gicdev, "revision", 3);
     qdev_prop_set_uint32(gicdev, "num-cpu", sc->num_cpus);
     qdev_prop_set_uint32(gicdev, "num-irq", AST2700_MAX_IRQ);

     redist_region_count = qlist_new();
     qlist_append_int(redist_region_count, sc->num_cpus);
     qdev_prop_set_array(gicdev, "redist-region-count", redist_region_count);

     if (!sysbus_realize(gicbusdev, errp)) {
         return false;
     }
     sysbus_mmio_map(gicbusdev, 0, sc->memmap[ASPEED_GIC_DIST]);
     sysbus_mmio_map(gicbusdev, 1, sc->memmap[ASPEED_GIC_REDIST]);

     for (i = 0; i < sc->num_cpus; i++) {
         DeviceState *cpudev = DEVICE(qemu_get_cpu(i));

Could we avoid qemu_get_cpu() and use the cpu array of the SoC instead ?

         int NUM_IRQS = 256, ARCH_GIC_MAINT_IRQ = 9, VIRTUAL_PMU_IRQ = 7;
         int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;

         const int timer_irq[] = {
             [GTIMER_PHYS] = 14,
             [GTIMER_VIRT] = 11,
             [GTIMER_HYP]  = 10,
             [GTIMER_SEC]  = 13,
         };
         int j;

         for (j = 0; j < ARRAY_SIZE(timer_irq); j++) {
             qdev_connect_gpio_out(cpudev, j,
                     qdev_get_gpio_in(gicdev, ppibase + timer_irq[j]));
         }

         qemu_irq irq = qdev_get_gpio_in(gicdev,
                                         ppibase + ARCH_GIC_MAINT_IRQ);
         qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt",
                                     0, irq);
         qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0,
                 qdev_get_gpio_in(gicdev, ppibase + VIRTUAL_PMU_IRQ));

         sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, 
ARM_CPU_IRQ));
         sysbus_connect_irq(gicbusdev, i + sc->num_cpus,
                            qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
         sysbus_connect_irq(gicbusdev, i + 2 * sc->num_cpus,
                            qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
         sysbus_connect_irq(gicbusdev, i + 3 * sc->num_cpus,
                            qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
     }

     return true;
}

struct Aspeed27x0SoCState {
     AspeedSoCState parent;

     ARMCPU cpu[ASPEED_CPUS_NUM];
     AspeedINTCState intc;
     GICv3State gic;
};

#define TYPE_ASPEED27X0_SOC "aspeed27x0-soc"
OBJECT_DECLARE_SIMPLE_TYPE(Aspeed27x0SoCState, ASPEED27X0_SOC)

Thanks,

C.