Hi Cedric, Andrew > From: Andrew Jeffery <and...@codeconstruct.com.au> > Sent: Thursday, January 30, 2025 11:22 AM > To: Cédric Le Goater <c...@kaod.org>; Jamin Lin <jamin_...@aspeedtech.com>; > Peter Maydell <peter.mayd...@linaro.org>; Steven Lee > <steven_...@aspeedtech.com>; Troy Lee <leet...@gmail.com>; Joel Stanley > <j...@jms.id.au>; open list:ASPEED BMCs <qemu-...@nongnu.org>; open > list:All patches CC here <qemu-devel@nongnu.org> > Cc: Troy Lee <troy_...@aspeedtech.com>; Yunlin Tang > <yunlin.t...@aspeedtech.com> > Subject: Re: [PATCH v1 01/18] hw/intc/aspeed: Rename INTC to INTC0 > > On Wed, 2025-01-29 at 18:03 +0100, Cédric Le Goater wrote: > > On 1/21/25 08:04, Jamin Lin wrote: > > > The design of the INTC has significant changes in the AST2700 A1. > > > In the > > > AST2700 A0, there was one INTC controller, whereas in the AST2700 > > > A1, there were two INTC controllers: INTC0 (CPU DIE) and INTC1 (I/O > > > DIE). > > > > > > The previous INTC model only supported the AST2700 A0 and was > > > implemented for the INTC0 (CPU DIE). To support the future INTC1 > > > (I/O DIE) model, rename INTC to INTC0. > > > > > > Why not introduce definitions with _INTC_IO_ and leave alone the > > current instead ? Do we expect to have more than 2 INTC controllers ? > > > > There was similar discussion on the devicetree bindings for the SCU a > while back: > > https://lore.kernel.org/all/94efc2d4ff280a112b869124fc9d7e35ac031596.cam > e...@codeconstruct.com.au/ > > Ryan didn't like deviating from their internal documentation :( > > Andrew
Thanks for your suggestion. Last year, Troy and I implemented the SCU(CPU Die) and SCU_IO(IO Die) models to support the AST2700. https://github.com/qemu/qemu/blob/master/hw/misc/aspeed_scu.c#L1073 https://github.com/qemu/qemu/blob/master/hw/misc/aspeed_scu.c#L1080 I am fine with keeping the INTC(CPU Die) naming and creating a new INTC_IO(IO Die) model to support the AST2700 A1. I have a question regarding the INTC_IO model implementation: Can I define separate "intc_io_class_init" and "intcio_class_realize" functions for INTC_IO, similar to the SCU/SCU_IO models? If yes, I think the patch “2 Support different memory region ops” can be omitted. Additionally, I suggest that both INTC and INTC_IO have their own MMIO callback functions, as their register addresses are different. Thanks-Jamin
