RE: [PATCH v5 7/9] aspeed/soc : Add AST1030 support

[Troy Lee]

Hi Philippe,

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Thursday, February 23, 2023 6:44 PM
> To: Jamin Lin <jamin_lin@aspeedtech.com>; Cédric Le Goater <clg@kaod.org>;
> Peter Maydell <peter.maydell@linaro.org>; Andrew Jeffery
> <andrew@aj.id.au>; Joel Stanley <joel@jms.id.au>; Alistair Francis
> <alistair@alistair23.me>; Cleber Rosa <crosa@redhat.com>; Wainer dos
> Santos Moschetta <wainersm@redhat.com>; Beraldo Leal
> <bleal@redhat.com>; open list:ASPEED BMCs <qemu-arm@nongnu.org>;
> open list:All patches CC here <qemu-devel@nongnu.org>
> Cc: Steven Lee <steven_lee@aspeedtech.com>; Troy Lee
> <troy_lee@aspeedtech.com>
> Subject: Re: [PATCH v5 7/9] aspeed/soc : Add AST1030 support
> 
> ping
> 
> On 29/12/22 12:16, Philippe Mathieu-Daudé wrote:
> > Hi,
> >
> > On 1/4/22 10:38, Jamin Lin wrote:
> >> From: Steven Lee <steven_lee@aspeedtech.com>
> >>
> >> The embedded core of AST1030 SoC is ARM Coretex M4.
> >> It is hard to be integrated in the common Aspeed Soc framework.
> >> We introduce a new ast1030 class with instance_init and realize
> >> handlers.
> >>
> >> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> >> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> >> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> >> ---
> >>   hw/arm/aspeed_ast10xx.c     | 299
> >> ++++++++++++++++++++++++++++++++++++
> >>   hw/arm/meson.build          |   6 +-
> >>   include/hw/arm/aspeed_soc.h |   3 +
> >>   3 files changed, 307 insertions(+), 1 deletion(-)
> >>   create mode 100644 hw/arm/aspeed_ast10xx.c
> >
> >
> >> +static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error
> >> **errp)
> >> +{
> >> +    AspeedSoCState *s = ASPEED_SOC(dev_soc);
> >> +    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
> >> +    MemoryRegion *system_memory = get_system_memory();
> >> +    DeviceState *armv7m;
> >> +    Error *err = NULL;
> >> +    int i;
> >> +
> >> +    if (!clock_has_source(s->sysclk)) {
> >> +        error_setg(errp, "sysclk clock must be wired up by the board
> >> code");
> >> +        return;
> >> +    }
> >> +
> >> +    /* General I/O memory space to catch all unimplemented device */
> >> +    create_unimplemented_device("aspeed.sbc",
> >> +                                sc->memmap[ASPEED_DEV_SBC],
> >> +                                0x40000);
> >> +    create_unimplemented_device("aspeed.io",
> >> +                                sc->memmap[ASPEED_DEV_IOMEM],
> >> +                                ASPEED_SOC_IOMEM_SIZE);
> >> +
> >> +    /* AST1030 CPU Core */
> >> +    armv7m = DEVICE(&s->armv7m);
> >> +    qdev_prop_set_uint32(armv7m, "num-irq", 256);
> >
> > Can you confirm this SoC has 256 and not 240 IRQs?
> >
The interrupt number start from 0 to 148, and some of them are reserved. 
Should we send an update to seth num-irq to 149?

Thanks,
Troy Lee

> >> +    qdev_prop_set_string(armv7m, "cpu-type", sc->cpu_type);
> >> +    qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
> >> +    object_property_set_link(OBJECT(&s->armv7m), "memory",
> >> +                             OBJECT(system_memory), &error_abort);
> >> +    sysbus_realize(SYS_BUS_DEVICE(&s->armv7m), &error_abort);
> >