Re: [PATCH] serial: 8250_bcm7271: improve bcm7271 8250 port

[Justin Chen]

On Sat, Aug 12, 2023 at 3:50 AM Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, Aug 11, 2023 at 03:14:01PM -0700, Justin Chen wrote:
> > The 8250 bcm7271 UART is not a direct match to PORT_16550A. The
> > Fifo is 32 and rxtrig values are {1, 8, 16, 30}. Create a PORT_BCM7271
> > to better capture the HW CAPS.
> >
> > Default the rxtrig level to 8.
> >
> > Signed-off-by: Justin Chen <justin.chen@xxxxxxxxxxxx>
> > ---
> >  drivers/tty/serial/8250/8250_bcm7271.c | 4 +---
> >  drivers/tty/serial/8250/8250_port.c    | 8 ++++++++
> >  include/uapi/linux/serial_core.h       | 3 +++
> >  3 files changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c
> > index d4b05d7ad9e8..aa5aff046756 100644
> > --- a/drivers/tty/serial/8250/8250_bcm7271.c
> > +++ b/drivers/tty/serial/8250/8250_bcm7271.c
> > @@ -1042,7 +1042,7 @@ static int brcmuart_probe(struct platform_device *pdev)
> >       dev_dbg(dev, "DMA is %senabled\n", priv->dma_enabled ? "" : "not ");
> >
> >       memset(&up, 0, sizeof(up));
> > -     up.port.type = PORT_16550A;
> > +     up.port.type = PORT_BCM7271;
> >       up.port.uartclk = clk_rate;
> >       up.port.dev = dev;
> >       up.port.mapbase = mapbase;
> > @@ -1056,8 +1056,6 @@ static int brcmuart_probe(struct platform_device *pdev)
> >               | UPF_FIXED_PORT | UPF_FIXED_TYPE;
> >       up.port.dev = dev;
> >       up.port.private_data = priv;
> > -     up.capabilities = UART_CAP_FIFO | UART_CAP_AFE;
> > -     up.port.fifosize = 32;
> >
> >       /* Check for a fixed line number */
> >       ret = of_alias_get_id(np, "serial");
> > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> > index 16aeb1420137..a6259a264041 100644
> > --- a/drivers/tty/serial/8250/8250_port.c
> > +++ b/drivers/tty/serial/8250/8250_port.c
> > @@ -322,6 +322,14 @@ static const struct serial8250_config uart_config[] = {
> >               .rxtrig_bytes   = {2, 66, 130, 194},
> >               .flags          = UART_CAP_FIFO,
> >       },
> > +     [PORT_BCM7271] = {
> > +             .name           = "bcm7271_uart",
> > +             .fifo_size      = 32,
> > +             .tx_loadsz      = 32,
> > +             .fcr            = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01,
> > +             .rxtrig_bytes   = {1, 8, 16, 30},
> > +             .flags          = UART_CAP_FIFO | UART_CAP_AFE
> > +     },
> >  };
> >
> >  /* Uart divisor latch read */
> > diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> > index 281fa286555c..369f845a3d1d 100644
> > --- a/include/uapi/linux/serial_core.h
> > +++ b/include/uapi/linux/serial_core.h
> > @@ -279,4 +279,7 @@
> >  /* Sunplus UART */
> >  #define PORT_SUNPLUS 123
> >
> > +/* Broadcom 7271 UART */
> > +#define PORT_BCM7271    124
>
> Why is this new id required?  What in userspace is going to use it and
> why can't the generic value be used instead?
>

I couldn't find a generic port that matches our FIFO size and
rxtrig_bytes. That is why I created a new one. Userspace currently
misreports what the rxtrig level is.

Thanks,
Justin

> thanks,
>
> greg k-h

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature