RE: [PATCH 3/4] serial: 8250: Add Aspeed UART driver

[ChiaWei Wang <chiawei_wang@aspeedtech.com>]

> From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Sent: Friday, February 10, 2023 9:52 PM
> 
> On Fri, 10 Feb 2023, Chia-Wei Wang wrote:
> 
> > Add the driver for Aspeed UART/VUART devices, which are 16550A
> > compatible. It is an wrapper to cover the generic 16550A operation
> > while exetending DMA feature for the devices.
> >
> > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com>
> > ---
> >  drivers/tty/serial/8250/8250_aspeed.c | 502
> ++++++++++++++++++++++++++
> >  drivers/tty/serial/8250/Kconfig       |   8 +
> >  drivers/tty/serial/8250/Makefile      |   1 +
> 
> Hi,
> 
> Before I look any further into this, could you please explain why this is made to
> be entirely separate from what we have in
>   drivers/tty/serial/8250/8250_aspeed_vuart.c
> ?
> 
> I quickly went through some functions and they've significant parts in common
> with no variations at all in many functions and you're defines are 1:1 too
> (except for the DMA buf sizes). It would seem much better to add the missing
> functionality into 8250_aspeed_vuart.c rather than creating something from
> scratch with large overlap with existing code.
> 
> If you intend to keep it as a separate one, you should have a rather good
> justification for it.

Yes, the main difference is the UART DMA support.
However, due to the UDMA design is not quite fit to the DMAEngine, the implementation is kind of hacking.
We thought leaving the original VUART driver unimpacted would be better.
The UDMA covers both UART and VUART DMA support, and so do the new 8250_aspeed.c.

Regards,
Chiawei