Skip to content

fix(kernel/mvebu-6.18): restore SysRq-via-BREAK on dw-apb-uart (8250_dw)#9760

Merged
igorpecovnik merged 1 commit intomainfrom
fix/sysrq-break-dw8250-mvebu
May 6, 2026
Merged

fix(kernel/mvebu-6.18): restore SysRq-via-BREAK on dw-apb-uart (8250_dw)#9760
igorpecovnik merged 1 commit intomainfrom
fix/sysrq-break-dw8250-mvebu

Conversation

@iav
Copy link
Copy Markdown
Contributor

@iav iav commented May 4, 2026

Summary

  • The mvebu kernel uses the same drivers/tty/serial/8250/8250_dw.c driver on Armada 38x as RK3399 does, and inherits the same SysRq-via-BREAK regression introduced by the guard(uart_port_lock_irqsave) refactor — which fix(kernel/rockchip64-7.0): restore SysRq-via-BREAK on dw-apb-uart (8250_dw) #9750 already fixes for kernel-rockchip64-7.0.
  • This patch is a verbatim copy of the same fix into patch/kernel/archive/mvebu-6.18/, applies cleanly (hunk context matches).

Without the patch, BREAK + sysrq-char is received 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.

Tests done

  • helios4 (Armada 388, kernel-mvebu 6.18-edge), picocom Ctrl-A 'C-\' h on the serial console: *** break sent *** followed immediately by [NNN.NNNNN][ C0] sysrq: HELP : loglevel(0-9) reboot(b) crash(c) ... (after raising console_loglevel above the boot-default loglevel=1, which otherwise filters KERN_INFO out of the console).
  • Diagnostic trace_printk patch (kept local in userpatches, not part of this PR) confirmed in the same kernel 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, and the only thing missing was the call site, which this patch restores.

Scope

Companion to #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.

Summary by CodeRabbit

  • Bug Fixes
    • Restored reliable SysRq handling on affected serial/UART consoles, fixing cases where BREAK→SysRq sequences could be lost.
    • Improved serial interrupt and processing behavior so emergency system commands and diagnostics entered via the serial console consistently reach the SysRq path.

@iav iav requested a review from Heisath as a code owner May 4, 2026 23:45
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 4, 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: c38f65a1-3b2d-4978-877c-4eef7d6e6132

📥 Commits

Reviewing files that changed from the base of the PR and between d39261f and fa30763.

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

📝 Walkthrough

Walkthrough

Replaces scoped guard(uart_port_lock_irqsave) usage with explicit uart_port_lock_irqsave(..., &flags) / uart_port_unlock_irqrestore(...) and calls uart_unlock_and_check_sysrq_irqrestore(...) at normal exits so SysRq handling runs correctly after BREAK→SysRq sequences in DW APB and generic 8250 IRQ handlers. (39 words)

Changes

SysRq and Lock Pattern Fix for 8250 UART Handlers

Layer / File(s) Summary
Early Exit Path Handling
drivers/tty/serial/8250/8250_dw.c
Replace scoped IRQ-save guard with uart_port_lock_irqsave(p, &flags) and add explicit uart_port_unlock_irqrestore(p, flags) on early returns for UART_IIR_NO_INT and UART_IIR_BUSY.
Core Interrupt Processing
drivers/tty/serial/8250/8250_dw.c
Hold explicit irqsave lock while calling serial8250_handle_irq_locked(p, iir) instead of relying on scoped guard semantics.
SysRq Check Integration
drivers/tty/serial/8250/8250_dw.c
After core handler returns, call uart_unlock_and_check_sysrq_irqrestore(p, flags) so SysRq handling is checked at the normal exit point.
Generic 8250 Handler Alignment
drivers/tty/serial/8250/8250_port.c
Mirror same explicit uart_port_lock_irqsave(port, &flags) usage and call uart_unlock_and_check_sysrq_irqrestore(port, flags) after serial8250_handle_irq_locked(port, iir) to keep behavior consistent.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nudged the locks where interrupts play,
Unwrapped the guards, let SysRq stay,
BREAK then check — no more surprise,
The serial hums beneath night skies,
A little hop, and all's okay.

🚥 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 pull request title clearly and specifically describes the main change: restoring SysRq-via-BREAK functionality for the dw-apb-uart driver in the mvebu-6.18 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
  • Commit unit tests in branch fix/sysrq-break-dw8250-mvebu

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

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

@github-actions github-actions Bot added 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, ... 05 Milestone: Second quarter release labels May 4, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@patch/kernel/archive/mvebu-6.18/general-serial-8250-fix-sysrq-break-dw-apb.patch`:
- Around line 6-8: Update the patch header text in
patch/kernel/archive/mvebu-6.18/general-serial-8250-fix-sysrq-break-dw-apb.patch
so it targets the mvebu backport: replace references to the Rockchip
reproduction path ("RK3399 / helios64", "Linux 7.0") with the validated mvebu
details ("helios4 / Armada 388") or add a clear note that this is a verbatim
copy of the Rockchip patch; make the same textual change for the repeated
description block mentioned in lines 26-32 so the archive history accurately
reflects the mvebu backport context.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f9c0ad4b-c504-43d1-a2fd-a24e16a834f8

📥 Commits

Reviewing files that changed from the base of the PR and between a6cb68a and 0c10436.

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

@iav iav force-pushed the fix/sysrq-break-dw8250-mvebu branch from 0c10436 to d39261f Compare May 5, 2026 00:05
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
patch/kernel/archive/mvebu-6.18/general-serial-8250-fix-sysrq-break-dw-apb.patch (1)

1-67: 💤 Low value

Patch header looks good; past description concern is resolved.

The new preamble (lines 6–16) explicitly identifies this as a verbatim copy from rockchip64-7.0 and records the mvebu validation details — this directly addresses the previous review note about misleading RK3399/helios64/Linux 7.0 references. The original description is clearly labeled as verbatim, leaving no ambiguity.

One minor nit: the patch is missing a Signed-off-by: Igor Velkov … line. Armbian downstream patches don't strictly require DCO sign-off, but it is conventional and helpful for audit trails.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@patch/kernel/archive/mvebu-6.18/general-serial-8250-fix-sysrq-break-dw-apb.patch`
around lines 1 - 67, The patch header is missing the conventional DCO
Signed-off-by line; add a "Signed-off-by: Igor Velkov
<325961+iav@users.noreply.github.com>" (exact string) to the patch commit
message/preamble for
patch/kernel/archive/mvebu-6.18/general-serial-8250-fix-sysrq-break-dw-apb.patch
so the commit metadata includes the author sign-off (place it after the existing
commit description block, keeping the verbatim original text intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@patch/kernel/archive/mvebu-6.18/general-serial-8250-fix-sysrq-break-dw-apb.patch`:
- Around line 1-67: The patch header is missing the conventional DCO
Signed-off-by line; add a "Signed-off-by: Igor Velkov
<325961+iav@users.noreply.github.com>" (exact string) to the patch commit
message/preamble for
patch/kernel/archive/mvebu-6.18/general-serial-8250-fix-sysrq-break-dw-apb.patch
so the commit metadata includes the author sign-off (place it after the existing
commit description block, keeping the verbatim original text intact).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 582a946a-1a50-4fb2-a765-dd11d4f4abc8

📥 Commits

Reviewing files that changed from the base of the PR and between 0c10436 and d39261f.

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

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 iav force-pushed the fix/sysrq-break-dw8250-mvebu branch from d39261f to fa30763 Compare May 5, 2026 00:24
@iav
Copy link
Copy Markdown
Contributor Author

iav commented May 5, 2026

@EvilOlaf ?

@github-actions github-actions Bot added the Ready to merge Reviewed, tested and ready for merge label May 5, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 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 5, 2026
@EvilOlaf
Copy link
Copy Markdown
Member

EvilOlaf commented May 6, 2026

I have no clue about mvebu nor do I have a helios4. Not sure what to say here.

@iav
Copy link
Copy Markdown
Contributor Author

iav commented May 6, 2026

@EvilOlaf Then I apologize for disturbing you.

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.

3 participants