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

[Cédric Le Goater]

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/arm64/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.

> It think that we can discuss it in a new AST2700 patch series.
Sure.

Thanks,

C.