Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Sep 30, 2025 at 11:03:29AM +0800, Binbin Zhou wrote:
> Hi Jiri:
> 
> Thanks for your reply.
> 
> On Mon, Sep 29, 2025 at 2:19 PM Jiri Slaby <jirislaby@kernel.org> wrote:
> >
> > Hi,
> >
> > On 28. 09. 25, 4:48, Binbin Zhou wrote:
> > > Hi Greg:
> > >
> > > Thanks for your reply.
> > >
> > > On Wed, Sep 24, 2025 at 6:22 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > >>
> > >> On Wed, Sep 24, 2025 at 02:29:37PM +0800, Binbin Zhou wrote:
> > >>> --- a/include/uapi/linux/serial_core.h
> > >>> +++ b/include/uapi/linux/serial_core.h
> > >>> @@ -31,6 +31,7 @@
> > >>>   #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
> > >>>   #define PORT_RT2880  29      /* Ralink RT2880 internal UART */
> > >>>   #define PORT_16550A_FSL64 30 /* Freescale 16550 UART with 64 FIFOs */
> > >>> +#define PORT_LOONGSON        31      /* Loongson 16550 UART */
> > >>
> > >> Why does userspace need to have this value exported?
> > >
> > > Sorry, this was a cheap mistake.
> > > It should follow the existing latest macro definition as follows:
> >
> > That was not the point. The point was why do you need that at all?
> 
> Emm...
> We attempted to define Loongson UART as a new `uart_config` entry.
> Therefore, `PORT_LOONGSON` is referenced as follows:
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c
> b/drivers/tty/serial/8250/8250_port.c
> index 719faf92aa8a..53efe841656f 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -310,6 +310,14 @@ static const struct serial8250_config uart_config[] = {
>                 .rxtrig_bytes   = {1, 8, 16, 30},
>                 .flags          = UART_CAP_FIFO | UART_CAP_AFE,
>         },
> +       [PORT_LOONGSON] = {
> +               .name           = "Loongson",
> +               .fifo_size      = 16,
> +               .tx_loadsz      = 16,
> +               .fcr            = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
> +               .rxtrig_bytes   = {1, 4, 8, 14},
> +               .flags          = UART_CAP_FIFO,
> +       },
>  };
> 
>  /* Uart divisor latch read */
> 
> Additionally, `uart.port.type` will also be assigned the value `PORT_LOONGSON`.

Yes, but that is the same exact value as PORT_PXA, which means this code
will not really work properly, right?  You can not just redefine an
already used user/kernel value.

thanks,

greg k-h