RE: [PATCH v0 1/2] aspeed: support uart controller both 0 and 1 base

[Jamin Lin]

> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Wednesday, February 7, 2024 1:00 AM
> To: Jamin Lin <jamin_lin@aspeedtech.com>; Peter Maydell
> <peter.maydell@linaro.org>; Andrew Jeffery <andrew@codeconstruct.com.au>;
> Joel Stanley <joel@jms.id.au>; open list:ASPEED BMCs
> <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_lee@aspeedtech.com>
> Subject: Re: [PATCH v0 1/2] aspeed: support uart controller both 0 and 1 base
> 
> On 2/6/24 04:29, Jamin Lin wrote:
> >> -----Original Message-----
> >> The uart definitions on the AST2700 are different :
> >>
> >>
> >> https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/arch/
> >> arm
> >> 64/boot/dts/aspeed/aspeed-g7.dtsi
> >>
> >>    serial0 = &uart0;
> >>    serial1 = &uart1;
> >>    serial2 = &uart2;
> >>    serial3 = &uart3;
> >>    serial4 = &uart4;
> >>    serial5 = &uart5;
> >>    serial6 = &uart6;
> >>    serial7 = &uart7;
> >>    serial8 = &uart8;
> >>           ...
> >>
> >> I think the names in the DT (and consequently in the QEMU models)
> >> follow the IP names in the datasheet.
> >>
> >> I don't think we care in QEMU, so I would be inclined to change the
> >> indexing of the device names in QEMU and start at 0, which would
> >> introduce a discrepancy for the AST2400, AST2600, AST2600 SoC.
> >>
> >> Let's see what the other maintainers have to say.
> >>
> >> Thanks,
> >>
> >> C.
> > Hi Cedric,
> >
> > Did you mean to change the naming of uart device to 0 base for all ASPEED
> SOCs?
> > If yes, it seems we need to do the following changes.
> > 1. add ASPEED_DEV_UART0 in aspeed_soc.h 2. Re-defined uart memory map
> > for ast2600, ast10x0, ast2500 and ast2400(uart0 -> ASPEED_DEV_UART0)
> > Take ast2600 for example:
> > static const hwaddr aspeed_soc_ast2600_memmap[] = {
> >      [ASPEED_DEV_UART1]     = 0x1E783000, --->
> [ASPEED_DEV_UART0]
> >      [ASPEED_DEV_UART2]     = 0x1E78D000, --->
> [ASPEED_DEV_UART1]
> >      [ASPEED_DEV_UART3]     = 0x1E78E000,
> >      [ASPEED_DEV_UART4]     = 0x1E78F000,
> >      [ASPEED_DEV_UART5]     = 0x1E784000,
> >      [ASPEED_DEV_UART6]     = 0x1E790000,
> >      [ASPEED_DEV_UART7]     = 0x1E790100,
> >      [ASPEED_DEV_UART8]     = 0x1E790200,
> >      [ASPEED_DEV_UART9]     = 0x1E790300,
> >      [ASPEED_DEV_UART10]    = 0x1E790400,
> >      [ASPEED_DEV_UART11]    = 0x1E790500,
> >      [ASPEED_DEV_UART12]    = 0x1E790600,
> >      [ASPEED_DEV_UART13]    = 0x1E790700, --->
> [ASPEED_DEV_UART12]
> > };
> > If no, could you please descript it more detail? So, I can change it and 
> > re-send
> this patch series.
> 
> Let's keep the datasheet names. I had forgotten the reason initially and from
> an HW POV it makes sense to keep them in sync. I will add some more
> comments to the patch.
> 
> > By the way, I will send a new patch series to support AST2700 in two weeks.
> > We encountered GIC issues. It seems that QEMU support GIC v3 but SPI did
> not support, yet.
> >
> >
> https://github.com/qemu/qemu/blob/master/hw/intc/arm_gicv3_dist.c#L383
> > https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/arch/a
> > rm64/boot/dts/aspeed/aspeed-g7.dtsi#L229
> 
> If you did any hacks or workarounds in the QEMU models, please keep them
> separate from the other patches so that we can discuss.
> 
Okay. Will do
Thanks-Jamin
> > It think that we can discuss it in a new AST2700 patch series.
> Sure.
> 
> Thanks,
> 
> C.
>