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

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: <CAHp75VdmzMCicN2FFoW3ANJYj2SDZBW9pbWkRm_knOgg8zQnHw@mail.gmail.com>
Date: Fri, 5 Dec 2025 10:03:11 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Crescent Hsieh <crescentcy.hsieh@...a.com>
Cc: gregkh@...uxfoundation.org, jirislaby@...nel.org, 
	ilpo.jarvinen@...ux.intel.com, linux-kernel@...r.kernel.org, 
	linux-serial@...r.kernel.org
Subject: Re: [PATCH v1 07/31] serial: 8250_mxupci: add GDL-based Rx routine
 for 8250_mxupci

On Fri, Dec 5, 2025 at 5:47 AM Crescent Hsieh <crescentcy.hsieh@...a.com> 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@...a.com> 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