Re: [PATCH v5 08/14] serial: 8250_mxpcie: speed up TX using memory-mapped FIFO window

[Andy Shevchenko]

On Mon, Aug 3, 2026 at 10:26 AM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
>
> On Mon, Aug 3, 2026 at 7:46 AM Jiri Slaby <jirislaby@xxxxxxxxxx> wrote:
> >
> > On 02. 08. 26, 11:10, Andy Shevchenko wrote:
> > > On Fri, Jul 31, 2026 at 10:49 AM Crescent Hsieh
> > > <crescentcy.hsieh@xxxxxxxx> wrote:
> > >>
> > >> The MUEx50 UART provides a memory-mapped TX FIFO data window along with
> > >> a TX FIFO level counter.
> > >>
> > >> Fill the TX FIFO in bulk via the MMIO FIFO window based on available
> > >> FIFO space, using uart_port_tx_limited() for the common serial-core TX
> > >> handling.
> > >
> > > ...
> > >
> > >> +static void mxpcie8250_tx_chars(struct uart_8250_port *up)
> > >> +{
> > >> +       struct uart_port *port = &up->port;
> > >> +       unsigned int offset = 0;
> > >> +       unsigned char c;
> > >> +
> > >> +       uart_port_tx_limited(port, c, port->fifosize - serial_in(up, MOXA_PUART_TX_FIFO_CNT),
> > >> +                            true,
> > >> +                            serial_out(up, MOXA_PUART_TX_FIFO_MEM + offset++, c),
> > >> +                            ({}));
> > >> +}
> > >
> > > At least three randomly chosen drivers use the empty tx_done, perhaps
> > > it's time to have a helper that embeds this ugly piece in it and hides
> > > from the driver?
> >
> > I haven't tried, can we make the helper a variadic macro?
>
> Since it's last argument it can be done with the variadic, but you can
> also reconsider the others from the same group (the wrappers on top of
> __uart_port_tx() call) to regroup argument list to make more than a
> single optional argument. This can mimic the case of
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/lib/kstrtox.h#n21
> where two optional arguments are possible.

Note, if you see the history of those changes, you can see how it can
be achieved iteratively without breaking the users and having a single
name of the function, say uart_port_tx() for a new approach.


-- 
With Best Regards,
Andy Shevchenko