On 2/4/25 07:50, Jamin Lin wrote:
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.
Good. I think this will reduce the changes and clarify the models.
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?
Looks OK to me.
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.
Do you mean the register offset in the MMIO aperture ? We try
to avoid duplication unless the code becomes too complex.
Please send a v2, splitting your series in 3 as requested in
the other email.
Thanks,
C.
