Skip to content

fix(kernel/rockchip64-7.0): restore SysRq-via-BREAK on dw-apb-uart (8250_dw)#9750

Merged
igorpecovnik merged 1 commit intoarmbian:mainfrom
iav:fix/sysrq-break-dw8250
May 5, 2026
Merged

fix(kernel/rockchip64-7.0): restore SysRq-via-BREAK on dw-apb-uart (8250_dw)#9750
igorpecovnik merged 1 commit intoarmbian:mainfrom
iav:fix/sysrq-break-dw8250

Conversation

@iav
Copy link
Copy Markdown
Contributor

@iav iav commented May 3, 2026

Summary

  • Restore SysRq-via-BREAK on dw-apb-uart (8250_dw) for linux-7.0.y kernels (RK3399 / helios64 and other rockchip64-7.0 boards).
  • The regression was introduced upstream in 8324a54f604d "serial: 8250: Add serial8250_handle_irq_locked()" (PATCH v2 4/7) and 883c5a2bc934 "serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling" (PATCH v2 5/7) by Ilpo Järvinen, both in the tty-7.0-rc5 pull (merge d46d5c838344); they replaced the explicit uart_unlock_and_check_sysrq_irqrestore() call with a guard(uart_port_lock_irqsave) scope. The guard releases the port lock at scope exit but never checks port->sysrq_ch, so handle_sysrq() is no longer called.
  • Other 8250-family drivers (amba-pl011, meson_uart, msm_serial, …) call uart_unlock_and_check_sysrq() explicitly; 8250_dw relied on serial8250_handle_irq() doing it for it. The fix restores that call on both paths.
  • The first commit was Cc: stable, so the regression also reaches v6.19.10 in the 6.19.y series.
  • Touches only patch/kernel/archive/rockchip64-7.0/ — no build-framework or extension changes.

Test plan

  • Verified on helios64 (RK3399, dw-apb-uart, ttyS2 @ 1.5 Mbaud, kernel 7.0.3-edge-rockchip64): on the picocom serial console, C-a C-\ (BREAK) followed by h produces sysrq: HELP : … from the kernel.
  • Reviewers welcome to test on other rockchip64-7.0 boards using dw-apb-uart.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed a regression preventing UART BREAK and SysRq character handling on dw-apb-uart devices in Linux 7.0.

…250_dw)

Sending a serial BREAK followed by a SysRq command character stopped
working on dw-apb-uart (e.g. RK3399 / helios64) in linux-7.0.y.

The refactor that introduced the regression replaced the explicit
uart_unlock_and_check_sysrq_irqrestore() in serial8250_handle_irq()
with a guard(uart_port_lock_irqsave) scope. The guard releases the
port lock at scope exit, but does NOT check port->sysrq_ch and
therefore never calls handle_sysrq().

Other 8250-family drivers (amba-pl011, meson_uart, msm_serial, ...)
each call uart_unlock_and_check_sysrq() explicitly in their IRQ
handler; 8250_dw relied on serial8250_handle_irq() doing it for it.

Fix: in dw8250_handle_irq() replace the guard() with explicit
uart_port_lock_irqsave() + uart_unlock_and_check_sysrq_irqrestore()
on the normal exit path; early-exit paths (UART_IIR_NO_INT /
UART_IIR_BUSY) get plain uart_port_unlock_irqrestore() since they
do not process RX chars.

The same fix is applied to serial8250_handle_irq() as
defence-in-depth for any other caller that goes through it.

Verified on helios64 (RK3399, dw-apb-uart, ttyS2 @ 1.5 Mbaud):
BREAK + 'h' on the picocom serial console produces the expected
'sysrq: HELP : ...' line from the kernel.

Assisted-by: Claude:claude-opus-4-7
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3b9a6cce-50f7-41de-b091-dd3f863bf5b4

📥 Commits

Reviewing files that changed from the base of the PR and between f13bc6b and e279755.

📒 Files selected for processing (1)
  • patch/kernel/archive/rockchip64-7.0/general-serial-8250-fix-sysrq-break-dw-apb.patch

📝 Walkthrough

Walkthrough

This patch fixes a SysRq BREAK handling regression in Linux 7.0's dw-apb-uart driver by refactoring IRQ handler locking patterns. Two functions now use explicit uart_port_lock_irqsave() and uart_unlock_and_check_sysrq_irqrestore() calls instead of scope-based guards to properly restore SysRq processing after handling interrupts.

Changes

SysRq BREAK Handling Fix in UART IRQ Handlers

Layer / File(s) Summary
Explicit Lock Acquisition
patch/kernel/archive/rockchip64-7.0/general-serial-8250-fix-sysrq-break-dw-apb.patch
Both dw8250_handle_irq() and serial8250_handle_irq() replace guard(uart_port_lock_irqsave) scope guards with explicit uart_port_lock_irqsave(port, &flags) calls to enable precise control over unlock points.
Early Return Path Fixes
patch/kernel/archive/rockchip64-7.0/general-serial-8250-fix-sysrq-break-dw-apb.patch
dw8250_handle_irq() adds explicit uart_port_unlock_irqrestore() calls on early exits (no interrupt, BUSY status) to ensure locks are released before returning.
SysRq Restoration on Unlock
patch/kernel/archive/rockchip64-7.0/general-serial-8250-fix-sysrq-break-dw-apb.patch
Both functions call uart_unlock_and_check_sysrq_irqrestore() at their normal exit paths after invoking serial8250_handle_irq_locked() to defer SysRq processing until lock release.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 A BREAK came through the serial line so fine,
SysRq whispered secrets, but lost the time.
With locks now explicit, and unlocks so clear,
The magic of checking returns, crystal near!
No more guard scope ghosts—just straightforward sight,
The dw-apb spins true, SysRq's alight! 🎯✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly describes the main change: restoring SysRq-via-BREAK functionality on dw-apb-uart devices for the rockchip64-7.0 kernel.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added 05 Milestone: Second quarter release size/medium PR with more then 50 and less then 250 lines Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... Patches Patches related to kernel, U-Boot, ... labels May 3, 2026
@github-actions github-actions Bot added the Ready to merge Reviewed, tested and ready for merge label May 4, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

✅ This PR has been reviewed and approved — all set for merge!

@github-actions github-actions Bot removed the Needs review Seeking for review label May 4, 2026
iav added a commit that referenced this pull request May 5, 2026
The mvebu kernel uses the same dw-apb-uart driver (drivers/tty/serial/8250/8250_dw.c)
on Armada 388 / 38x SoCs as RK3399 does, and inherits the same SysRq-via-BREAK
regression introduced by the guard(uart_port_lock_irqsave) refactor that PR #9750
fixes for kernel-rockchip64-7.0.

Verified on helios4 (Armada 388, kernel-mvebu 6.18-edge) 2026-05-05:
  - Without patch: BREAK + 'h' through picocom is consumed by the IRQ handler
    (port->sysrq_ch is latched correctly), but uart_unlock_and_check_sysrq_irqrestore()
    is never called from the handler tail, so handle_sysrq() never runs.
  - With patch: BREAK + 'h' produces 'sysrq: HELP : ...' on the serial console
    (after raising console_loglevel above the boot-default 1).
  - Diagnostic trace_printk patch (local userpatches, not part of this PR)
    confirmed that has_sysrq=1, sysrq_armed=jiffies+5HZ after BREAK,
    sysrq_ch=104 ('h'=0x68) after the next char — i.e. all preconditions for
    handle_sysrq() are met, the only thing missing was the call site, which
    this patch restores.

This is a verbatim copy of the rockchip64-7.0 patch (PR #9750) into
patch/kernel/archive/mvebu-6.18/. Hunk context matches, applies cleanly.

Per-board impact is identical to the original PR #9750: any mvebu board whose
serial console is dw-apb-uart-derived (Armada 38x family — helios4, espressobin,
clearfog, etc.) gets working sysrq via BREAK back.

Companion to PR #9750 (rockchip64-7.0). No upstream submission planned (per
maintainer decision in #9750 thread).

Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
iav added a commit that referenced this pull request May 5, 2026
The mvebu kernel uses the same dw-apb-uart driver (drivers/tty/serial/8250/8250_dw.c)
on Armada 388 / 38x SoCs as RK3399 does, and inherits the same SysRq-via-BREAK
regression introduced by the guard(uart_port_lock_irqsave) refactor that PR #9750
fixes for kernel-rockchip64-7.0.

Verified on helios4 (Armada 388, kernel-mvebu 6.18-edge) 2026-05-05:
  - Without patch: BREAK + 'h' through picocom is consumed by the IRQ handler
    (port->sysrq_ch is latched correctly), but uart_unlock_and_check_sysrq_irqrestore()
    is never called from the handler tail, so handle_sysrq() never runs.
  - With patch: BREAK + 'h' produces 'sysrq: HELP : ...' on the serial console
    (after raising console_loglevel above the boot-default 1).
  - Diagnostic trace_printk patch (local userpatches, not part of this PR)
    confirmed that has_sysrq=1, sysrq_armed=jiffies+5HZ after BREAK,
    sysrq_ch=104 ('h'=0x68) after the next char — i.e. all preconditions for
    handle_sysrq() are met, the only thing missing was the call site, which
    this patch restores.

This is a verbatim copy of the rockchip64-7.0 patch (PR #9750) into
patch/kernel/archive/mvebu-6.18/. Hunk context matches, applies cleanly.

Per-board impact is identical to the original PR #9750: any mvebu board whose
serial console is dw-apb-uart-derived (Armada 38x family — helios4, espressobin,
clearfog, etc.) gets working sysrq via BREAK back.

Companion to PR #9750 (rockchip64-7.0). No upstream submission planned (per
maintainer decision in #9750 thread).

Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
@igorpecovnik igorpecovnik merged commit ad8f89d into armbian:main May 5, 2026
12 checks passed
@iav iav deleted the fix/sysrq-break-dw8250 branch May 5, 2026 15:03
igorpecovnik pushed a commit that referenced this pull request May 6, 2026
The mvebu kernel uses the same dw-apb-uart driver (drivers/tty/serial/8250/8250_dw.c)
on Armada 388 / 38x SoCs as RK3399 does, and inherits the same SysRq-via-BREAK
regression introduced by the guard(uart_port_lock_irqsave) refactor that PR #9750
fixes for kernel-rockchip64-7.0.

Verified on helios4 (Armada 388, kernel-mvebu 6.18-edge) 2026-05-05:
  - Without patch: BREAK + 'h' through picocom is consumed by the IRQ handler
    (port->sysrq_ch is latched correctly), but uart_unlock_and_check_sysrq_irqrestore()
    is never called from the handler tail, so handle_sysrq() never runs.
  - With patch: BREAK + 'h' produces 'sysrq: HELP : ...' on the serial console
    (after raising console_loglevel above the boot-default 1).
  - Diagnostic trace_printk patch (local userpatches, not part of this PR)
    confirmed that has_sysrq=1, sysrq_armed=jiffies+5HZ after BREAK,
    sysrq_ch=104 ('h'=0x68) after the next char — i.e. all preconditions for
    handle_sysrq() are met, the only thing missing was the call site, which
    this patch restores.

This is a verbatim copy of the rockchip64-7.0 patch (PR #9750) into
patch/kernel/archive/mvebu-6.18/. Hunk context matches, applies cleanly.

Per-board impact is identical to the original PR #9750: any mvebu board whose
serial console is dw-apb-uart-derived (Armada 38x family — helios4, espressobin,
clearfog, etc.) gets working sysrq via BREAK back.

Companion to PR #9750 (rockchip64-7.0). No upstream submission planned (per
maintainer decision in #9750 thread).

Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

05 Milestone: Second quarter release Hardware Hardware related like kernel, U-Boot, ... Patches Patches related to kernel, U-Boot, ... Ready to merge Reviewed, tested and ready for merge size/medium PR with more then 50 and less then 250 lines

Development

Successfully merging this pull request may close these issues.

2 participants