Re: [PATCH v1 07/31] serial: 8250_mxupci: add GDL-based Rx routine for 8250_mxupci

[Andy Shevchenko]

On Fri, Dec 5, 2025 at 5:47 AM Crescent Hsieh <crescentcy.hsieh@xxxxxxxx> wrote:
> On Sun, Nov 30, 2025 at 07:29:47PM +0200, Andy Shevchenko wrote:
> > On Sun, Nov 30, 2025 at 12:43 PM Crescent Hsieh
> > <crescentcy.hsieh@xxxxxxxx> wrote:
> > > -       if (lsr & (UART_LSR_DR | UART_LSR_BI) && !skip_rx)
> > > -               lsr = serial8250_rx_chars(up, lsr);
> > > -
> > > +       if (lsr & (UART_LSR_DR | UART_LSR_BI) && !skip_rx) {
> > > +               if (lsr & UART_LSR_BRK_ERROR_BITS)
> > > +                       lsr = serial8250_rx_chars(up, lsr);
> > > +               else
> > > +                       mxupci8250_rx_chars(up);
> > > +       }
> >
> > Oh, can we reduce ping-pong a bit (the modification of the lines just
> > being added earlier in the same patch series)?
> >
> > I think you can create a helper to wrap 8250_rx_chars() with split
> > version of the almost unreadable conditionals, this will also remove
> > the skip_rx variable
>
> I want to confirm whether I understood your suggestion correctly:
>
> Should I first introduce a small wrapper function for Rx processing,
> move all the complicated Rx conditions into that wrapper so that
> handle_irq() remains short and readable, and then, in a later patch,
> introduce the actual custom Rx routine and update the wrapper to call
> it?

Yes, something like

old_rx() {}
new_rx() {}

if (foo)
  old_rx()
else
 new_rx()

should be in the result.

-- 
With Best Regards,
Andy Shevchenko