Re: [PATCH 6.12.y 0/8] serial: 8250_dw: backport BUSY deassert series

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 6427 bytes --]

On Sun, 10 May 2026, Ionut Nechita (Wind River) wrote:

> From: Ionut Nechita <ionut.nechita@windriver.com>
> 
> Hi Greg, Ilpo,
> 
> This series backports the 8250_dw BUSY deassert fix to 6.12.y, per
> Ilpo's guidance from the request thread:
> 
>   https://lore.kernel.org/linux-serial/deb9499d-3245-7e38-9034-e533d4b5f512@linux.intel.com/
> 
> Background: we ship 6.12 LTS as part of a certified production
> platform (StarlingX/Yocto) and are hitting the BUSY assertion issue
> on Intel DesignWare 8250 UARTs - LCR writes get silently ignored
> under Rx load, causing baud-rate / framing mismatches after
> set_termios. A major LTS bump is a multi-month re-qualification we
> can't justify for a single subsystem fix.
> 
> The original mainline series is 7 patches by Ilpo (plus its
> dependencies). For 6.12.y, the resulting series here is 8 patches:
> 
>   Prerequisites (per Ilpo):
>    1/8 serial: 8250: use serial_port_in/out() helpers       [dbd26a886e94]
>    2/8 serial: 8250_dw: Comment possible corner cases ...   [bd8cad85561b]
> 
>   BUSY deassert series:
>    3/8 serial: 8250: Protect LCR write in shutdown          [59a33d83bbe6]
>    4/8 serial: 8250_dw: Avoid unnecessary LCR writes        [8002d6d6d0d8]
>    5/8 serial: 8250: Add serial8250_handle_irq_locked()     [8324a54f604d]
>    6/8 serial: 8250_dw: Rework dw8250_handle_irq() ...      [883c5a2bc934]
>    7/8 serial: 8250_dw: Rework IIR_NO_INT handling ...      [73a4ed8f9efa]
>    8/8 serial: 8250_dw: Ensure BUSY is deasserted           [a7b9ce39fbe4]
> 
> Notes:
> 
>  - Patch 6/7 of the original mainline series,
>    commit e0a368ae7953 ("serial: 8250: Add late synchronize_irq() to shutdown to handle DW UART BUSY"),
>    is *already* in 6.12.y as
>    commit 0bae1c670aa8 ("serial: 8250: Add late synchronize_irq() to shutdown to handle DW UART BUSY")
>    (Ilpo, 2026-02-03), so it is not re-sent here. Functionally this
>    means patches 3-8 above land on top of the existing
>    late-synchronize_irq() fix; the conflict in patch 3 (LCR write
>    placement around the late synchronize_irq) was resolved
>    accordingly.
> 
>  - Ilpo's other suggested prerequisites are *not* included as prereqs:
>      * commit b339809edda1 ("serial: 8250: use guard()s")
>      * commit fc9ceb501e38 ("serial: 8250: sanitize uart_port::serial_{in,out}() types")
>      * commit c213375e3283 ("serial: 8250_dw: Call dw8250_quirks() conditionally")
> 
>    Reasoning:
> 
>      * commit b339809edda1 ("serial: 8250: use guard()s") is a
>        large refactor and only the shutdown hunk was needed;
>        instead, patches 3, 5, 6, 7 here are adapted to use the
>        existing explicit uart_port_lock_irqsave/unlock_irqrestore
>        form rather than the cleanup-based guard() introduced by
>        that commit;

Hi,

By doing this you had to add some gotos which makes code control flow 
more complicated than it was in the original changes. From reviewer's 
point of view, the code is more complicated without guard() than with 
them.

guard() seems to be already used in 6.12 in general so I don't know why 
you had to go replacing guard()s introduced in the original changes with 
lock/unlock pairs.

FWIW, I scanned through the patches and didn't find anything that 
interesting (other than that the approach chosen ignored my advice that 
would have resulted in less changes/conflicts compared with the original 
changes).

>      * commit fc9ceb501e38 ("serial: 8250: sanitize
>        uart_port::serial_{in,out}() types") only causes a trivial
>        conflict in patch 8 in code that the BUSY change removes
>        anyway (per Ilpo's note);
>      * commit c213375e3283 ("serial: 8250_dw: Call dw8250_quirks()
>        conditionally") only causes a conflict in patch 7 around
>        the dw8250_setup_dma_filter() helper and the conditional
>        p->handle_irq assignment, neither of which exist in 6.12.y
>        and neither of which is needed for the BUSY fix.
> 
>  - Namespace export syntax: in 6.12.y both EXPORT_SYMBOL_NS_GPL()
>    and MODULE_IMPORT_NS() apply __stringify(ns) to the namespace
>    argument, so it must be a bare identifier. Mainline (where the
>    upstream patches were written) accepts a string literal. Patches
>    5 and 8 here use the bare-identifier form (SERIAL_8250) instead
>    of the upstream string form ("SERIAL_8250"); without this fix
>    the .vmlinux.export.c link step fails with "expected ':' or ')'
>    before 'SERIAL_8250'". This is noted in the [Ionut: ...] block
>    of the affected patches.
> 
>    Each of patches 3, 5, 6, 7 and 8 carries an explicit
>    "[Ionut: adapt to 6.12.y - ...]" note describing exactly what
>    was changed relative to the upstream commit.
> 
> Build:
>  - Each patch builds individually on 6.12.87 to a complete vmlinux
>    (bisect-safe), with CONFIG_SERIAL_8250=y, CONFIG_SERIAL_8250_DW=m
>    on x86_64 defconfig.
> 
> Testing plan:
>  - We will test on Intel platforms with DW APB UART
>    (snps,dw-apb-uart) running 6.12.57-rt / 6.12.87-rt (PREEMPT_RT)
>    to confirm the original symptom (LCR writes silently ignored
>    under Rx load -> baud / framing mismatch after set_termios) is
>    gone. Will report Tested-by once cycles complete.

Testing with a reproduces is highly appreciated.

> Based on: linux-6.12.y at v6.12.87 (8bf2f55ef536).
> 
> Andy Shevchenko (1):
>   serial: 8250_dw: Comment possible corner cases in serial_out()
>     implementation
> 
> Ilpo Järvinen (6):
>   serial: 8250: Protect LCR write in shutdown
>   serial: 8250_dw: Avoid unnecessary LCR writes
>   serial: 8250: Add serial8250_handle_irq_locked()
>   serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling
>   serial: 8250_dw: Rework IIR_NO_INT handling to stop interrupt storm
>   serial: 8250_dw: Ensure BUSY is deasserted
> 
> Jiri Slaby (SUSE) (1):
>   serial: 8250: use serial_port_in/out() helpers
> 
>  drivers/tty/serial/8250/8250.h      |  25 +++
>  drivers/tty/serial/8250/8250_dw.c   | 298 +++++++++++++++++++++++-----
>  drivers/tty/serial/8250/8250_fsl.c  |   8 +-
>  drivers/tty/serial/8250/8250_omap.c |   2 +-
>  drivers/tty/serial/8250/8250_port.c |  66 +++---
>  include/linux/serial_8250.h         |   1 +
>  6 files changed, 319 insertions(+), 81 deletions(-)
> 
> 

-- 
 i.