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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <KL1PR0601MB378160E42842A0A55947BF7191DD9@KL1PR0601MB3781.apcprd06.prod.outlook.com>
Date:   Mon, 13 Feb 2023 01:45:05 +0000
From:   ChiaWei Wang <chiawei_wang@...eedtech.com>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "joel@....id.au" <joel@....id.au>,
        "andrew@...id.au" <andrew@...id.au>,
        Jiri Slaby <jirislaby@...nel.org>,
        linux-serial <linux-serial@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "openbmc@...ts.ozlabs.org" <openbmc@...ts.ozlabs.org>
Subject: RE: [PATCH 3/4] serial: 8250: Add Aspeed UART driver

> From: Ilpo Järvinen <ilpo.jarvinen@...ux.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@...eedtech.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