> -----Original Message----- > From: Philippe Mathieu-Daudé <phi...@linaro.org> > Sent: Monday, February 5, 2024 9:16 PM > To: Cédric Le Goater <c...@kaod.org>; Jamin Lin <jamin_...@aspeedtech.com>; > Peter Maydell <peter.mayd...@linaro.org>; Andrew Jeffery > <and...@codeconstruct.com.au>; 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> > Subject: Re: [PATCH v0 1/2] aspeed: support uart controller both 0 and 1 base > > On 5/2/24 11:46, Cédric Le Goater wrote: > > Hello Jamin, > > > > On 2/5/24 10:14, Jamin Lin wrote: > >> According to the design of ASPEED SOCS, the uart controller is 1 base > >> for ast10x0, ast2600, ast2500 and ast2400. > >> > >> However, the uart controller is 0 base for ast2700. > >> To support uart controller both 0 and 1 base, adds uasrt_bases > >> parameter in AspeedSoCClass and set the default uart controller 1 > >> base for ast10x0, astt2600, ast2500 and ast2400. > > > > The board definition can set 'amc->uart_default' to choose a different > > default serial port for the console, or use the "bmc-console" machine > > option . Isn't it enough ? May be I am misunderstanding the need. > > > > To clarify, > > > > ASPEED_DEV_UART1 is in the first serial port on the boards. > > > > I think we chose to start the indexing at 1 because the Aspeed QEMU > > modeling began first with the UART model (console) and for simplicity, > > we copied the definitions of the device tree from Linux : > > > > serial0 = &uart1; > > serial1 = &uart2; > > serial2 = &uart3; > > serial3 = &uart4; > > serial4 = &uart5; > > serial5 = &vuart; > > > > We replicated this indexing starting at 1 to nearly all device models : > > > > ASPEED_DEV_UART1 - 13 > > ASPEED_DEV_SPI1 -2 > > ASPEED_DEV_EHCI1 -2 > > ASPEED_DEV_TIMER1 - 8 > > ASPEED_DEV_ETH1 -4 > > ASPEED_DEV_MII1 - 4 > > ASPEED_DEV_JTAG0 - 1 <--- !! > > ASPEED_DEV_FSI1 - 2 > > > > I don't know what would be ASPEED_DEV_UART0 in this context. > > > > May be you could send a simplified AST2700 SoC model with definitions > > of a minimum address space and IRQ space ? > > Looking at TF-A definitions, > https://github.com/ARM-software/arm-trusted-firmware/commit/85f199b774 > 47 > > #define UART_BASE U(0x14c33000) > #define UART12_BASE (UART_BASE + 0xb00) > #define CONSOLE_UART_BASE UART12_BASE > > As Cédric described, we have TF-A UART12_BASE -> QEMU > ASPEED_DEV_UART13. As Cédric described, the UART definitions on the AST2700 are different : https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/arch/arm64/boot/dts/aspeed/aspeed-g7.dtsi
serial0 = &uart0; serial1 = &uart1; serial2 = &uart2; serial3 = &uart3; serial4 = &uart4; serial5 = &uart5; serial6 = &uart6; serial7 = &uart7; serial8 = &uart8; According to the current design of ASPEED QEMU UART model, it used "1" base device name and Follow the IP names in the datasheet Take ast2600 for example: static const hwaddr aspeed_soc_ast2600_memmap[] = { [ASPEED_DEV_UART1] = 0x1E783000, } AST2600 datashee description: Base Address of UART1 = 0x1E78 3000 Base Address of UART2 = 0x1E78 D000 Base Address of UART3 = 0x1E78 E000 Base Address of UART4 = 0x1E78 F000 Base Address of UART5 = 0x1E78 4000 Base Address of UART6 = 0x1E79 0000 However, device name of uart controller had been changed to "0" base for ast2700. If we want to control uart0(ASPEED_DEV_UART1), we should set the memory map as following, static const hwaddr aspeed_soc_ast2700_memmap[] = { [ASPEED_DEV_UART1] = 0X14C33000, [ASPEED_DEV_UART2] = 0X14C33100, [ASPEED_DEV_UART3] = 0X14C33200, [ASPEED_DEV_UART4] = 0X14C33300, [ASPEED_DEV_UART5] = 0X12C1A000, } AST2700 datasheet description: AST2700 integrate 13 sets of UART. Base Address of UART0 = 0x14C3_3000 Base Address of UART1 = 0x14C3_3100 Base Address of UART2 = 0x14C3_3200 Base Address of UART3 = 0x14C3_3300 Base Address of UART4 = 0x12C1_A000 Base Address of UART5 = 0x14C3_3400 Base Address of UART6 = 0x14C3_3500 Base Address of UART7 = 0x14C3_3600 Base Address of UART8 = 0x14C3_3700 Base Address of UART9 = 0x14C3_3800 Base Address of UART10 = 0x14C3_3900 Base Address of UART11 = 0x14C3_3A00 Base Address of UART12 = 0x14C3_3B00 As you said, uart12 mapped ASPEED_DEV_UART13. The device naming will confuse users because the device name in qemu mismatch with ast2700 datasheet. That way why we want to add ASPEED_DEV_UART0 and set the memory map of AST2700 as following. static const hwaddr aspeed_soc_ast2700_memmap[] = { [ASPEED_DEV_UART0] = 0X14C33000, [ASPEED_DEV_UART1] = 0X14C33100, [ASPEED_DEV_UART2] = 0X14C33200, [ASPEED_DEV_UART3] = 0X14C33300, [ASPEED_DEV_UART4] = 0X12C1A000, [ASPEED_DEV_UART5] = 0X14C33400, [ASPEED_DEV_UART6] = 0X14C33500, [ASPEED_DEV_UART7] = 0X14C33600, [ASPEED_DEV_UART8] = 0X14C33700, [ASPEED_DEV_UART9] = 0X14C33800, [ASPEED_DEV_UART10] = 0X14C33900, [ASPEED_DEV_UART11] = 0X14C33A00, [ASPEED_DEV_UART12] = 0X14C33B00, } Thanks-Jamin