* [PATCH] tty: serial: 8250: protect against NULL uart->port.dev in register
From: Stepan Ionichev @ 2026-05-08 18:12 UTC (permalink / raw)
To: gregkh; +Cc: jirislaby, linux-serial, linux-kernel, Stepan Ionichev
serial8250_register_8250_port() conditionally copies uart->port.dev
from up->port.dev only when up->port.dev is non-NULL:
if (up->port.dev) {
uart->port.dev = up->port.dev;
...
}
So if both the existing uart slot and up have a NULL ->dev,
uart->port.dev remains NULL. The very next ACPI companion check
then dereferences it unconditionally:
if (!has_acpi_companion(uart->port.dev)) {
has_acpi_companion() reads dev->fwnode without a NULL guard
(include/linux/acpi.h), so this NULL-derefs the kernel for the
remaining no-dev case rather than just skipping the
mctrl_gpio_init() initialisation as intended.
smatch flags the inconsistency:
drivers/tty/serial/8250/8250_core.c:767
serial8250_register_8250_port() error: 'uart->port.dev' could be
null (see line 719)
Guard the call with a NULL check so register continues to work
for callers that legitimately have no parent device (legacy
non-OF/non-ACPI registrations).
No functional change for callers that pass a non-NULL ->dev.
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
drivers/tty/serial/8250/8250_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e8893..e136cec0c 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -764,7 +764,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
* Only call mctrl_gpio_init(), if the device has no ACPI
* companion device
*/
- if (!has_acpi_companion(uart->port.dev)) {
+ if (uart->port.dev && !has_acpi_companion(uart->port.dev)) {
struct mctrl_gpios *gpios = mctrl_gpio_init(&uart->port, 0);
if (IS_ERR(gpios)) {
ret = PTR_ERR(gpios);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v1 1/2] serial: qcom-geni: trace: Add tracepoint support for Qualcomm GENI serial
From: Praveen Talari @ 2026-05-09 1:19 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, Greg Kroah-Hartman,
Jiri Slaby, Konrad Dybcio, linux-kernel, linux-trace-kernel,
linux-arm-msm, linux-serial, Mukesh Kumar Savaliya,
Aniket Randive, chandana.chiluveru, jyothi.seerapu
In-Reply-To: <20260508132543.4f100ae0@gandalf.local.home>
On 08-05-2026 22:55, Steven Rostedt wrote:
> On Wed, 06 May 2026 22:54:44 +0530
> Praveen Talari <praveen.talari@oss.qualcomm.com> wrote:
>
>> +TRACE_EVENT(geni_serial_tx_data,
>> + TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
>> + TP_ARGS(dev, buf, len),
>> +
>> + TP_STRUCT__entry(__string(name, dev_name(dev))
>> + __field(unsigned int, len)
>> + __dynamic_array(u8, data, len)
>> + ),
>> +
>> + TP_fast_assign(__assign_str(name);
>> + __entry->len = len;
>> + memcpy(__get_dynamic_array(data), buf, len);
>> + ),
>> +
>> + TP_printk("%s: tx_len=%u data=%s",
>> + __get_str(name), __entry->len,
>> + __print_hex(__get_dynamic_array(data), __entry->len))
>> +);
>> +
>> +TRACE_EVENT(geni_serial_rx_data,
>> + TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
>> + TP_ARGS(dev, buf, len),
>> +
>> + TP_STRUCT__entry(__string(name, dev_name(dev))
>> + __field(unsigned int, len)
>> + __dynamic_array(u8, data, len)
>> + ),
>> +
>> + TP_fast_assign(__assign_str(name);
>> + __entry->len = len;
>> + memcpy(__get_dynamic_array(data), buf, len);
>> + ),
>> +
>> + TP_printk("%s: rx_len=%u data=%s",
> Do you really need to say "tx_len" and "rx_len", could it just be "len" and
> have the name of the tracepoint show what it is?
Sure. I will review and update next patch.
>
> Each TRACE_EVENT() is really just a:
>
> DECLARE_EVENT_CLASS() followed by a DEFINE_EVENT()
>
> underneath.
>
> And each TRACE_EVENT() costs around 5K in size, where most of that is in
> the DECLARE_EVENT_CLASS() portion. Thus, you can save some memory by using
> DECLARE_EVENT_CLASS() and then define the above two events with
> DEFINE_EVENT().
Thank you for suggestion and will update in next patch.
Thanks,
Praveen Talari
>
> -- Steve
>
>
>> + __get_str(name), __entry->len,
>> + __print_hex(__get_dynamic_array(data), __entry->len))
>> +);
>> +
^ permalink raw reply
* [PATCH] tty: n_tty: read termios under lock in poll
From: Cen Zhang @ 2026-05-10 2:59 UTC (permalink / raw)
To: gregkh, jirislaby; +Cc: linux-kernel, linux-serial, baijiaju1990, Cen Zhang
n_tty_poll() uses input_available_p() to decide whether buffered input
makes the tty readable. That helper reads termios state through
L_EXTPROC(), VMIN, and VTIME, but the poll path does not hold the read
side of tty->termios_rwsem.
tty_set_termios() updates tty->termios under the write side of the same
semaphore, including c_lflag and c_cc[]. n_tty_read() already takes the
read side before reading the same termios fields and before calling
input_available_p(). Protect the poll-side readiness checks the same way
so poll observes a coherent termios state when deciding whether to report
readable input.
Do not hold termios_rwsem across tty_buffer_flush_work(), matching the
read path which drops the semaphore before flushing pending receive work
and then checks input availability again after reacquiring it.
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
drivers/tty/n_tty.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index e6a0f5b40d0a..c8e1882782db 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2437,13 +2437,17 @@ static __poll_t n_tty_poll(struct tty_struct *tty, struct file *file,
poll_wait(file, &tty->read_wait, wait);
poll_wait(file, &tty->write_wait, wait);
- if (input_available_p(tty, 1))
- mask |= EPOLLIN | EPOLLRDNORM;
- else {
- tty_buffer_flush_work(tty->port);
+ scoped_guard(rwsem_read, &tty->termios_rwsem) {
if (input_available_p(tty, 1))
mask |= EPOLLIN | EPOLLRDNORM;
}
+ if (!(mask & (EPOLLIN | EPOLLRDNORM))) {
+ tty_buffer_flush_work(tty->port);
+ scoped_guard(rwsem_read, &tty->termios_rwsem) {
+ if (input_available_p(tty, 1))
+ mask |= EPOLLIN | EPOLLRDNORM;
+ }
+ }
if (tty->ctrl.packet && tty->link->ctrl.pktstatus)
mask |= EPOLLPRI | EPOLLIN | EPOLLRDNORM;
if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
--
2.43.0
^ permalink raw reply related
* [PATCH] vt: keyboard: serialize KDGETLED with keyboard_tasklet
From: Cen Zhang @ 2026-05-10 3:13 UTC (permalink / raw)
To: gregkh, jirislaby; +Cc: linux-kernel, linux-serial, baijiaju1990, Cen Zhang
KDGETLED reaches getledstate() and samples ledstate without serializing
against keyboard_tasklet.
That is problematic because kbd_bh() temporarily stores ~leds into
ledstate during a VT switch to force LED propagation before committing
the final value. An unlocked KDGETLED can therefore observe that
internal sentinel instead of the last committed LED state.
Other ledstate readers in this file already quiesce keyboard_tasklet
before sampling the value. Do the same in getledstate() so the ioctl
only returns a stable snapshot.
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
drivers/tty/vt/keyboard.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index 13bc048f45e86..671ad4a5cb64c 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -1127,7 +1127,13 @@ static void kbd_init_leds(void)
*/
static unsigned char getledstate(void)
{
- return ledstate & 0xff;
+ unsigned char leds;
+
+ tasklet_disable(&keyboard_tasklet);
+ leds = ledstate & 0xff;
+ tasklet_enable(&keyboard_tasklet);
+
+ return leds;
}
void setledstate(struct kbd_struct *kb, unsigned int led)
--
2.43.0
^ permalink raw reply related
* [PATCH] ARM: footbridge: convert to sparse IRQs
From: Ethan Nelson-Moore @ 2026-05-10 5:20 UTC (permalink / raw)
To: linux-arm-kernel, linux-input, linux-serial
Cc: Ethan Nelson-Moore, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, Dmitry Torokhov, Jiri Slaby,
Russell King (Oracle), Linus Walleij, Kees Cook,
Nathan Chancellor, Sebastian Andrzej Siewior, Steven Rostedt,
Thomas Weissschuh, Peter Zijlstra
To improve future maintainability, change the interrupt handling for
mach-footbridge to use sparse IRQs.
Since the number of possible interrupts is already fixed and relatively
small, just make it use all legacy interrupts preallocated using the
.nr_irqs field in the machine descriptor, rather than actually
allocating domains on the fly.
Many files had to be adjusted to include <mach/irqs.h>
explicitly because it is no longer implicitly included with sparse
IRQs.
Description adapted from commit c78a41fc04f0 ("ARM: s3c24xx: convert
to sparse-irq").
Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
---
This commit depends on my previous submission "ARM: <asm/floppy.h>: fix
build with sparse IRQs".
arch/arm/Kconfig | 2 +-
arch/arm/include/asm/irq.h | 4 +++-
| 2 +-
| 2 +-
| 2 +-
| 2 ++
| 4 +---
| 2 +-
| 2 +-
| 2 +-
| 2 ++
| 2 +-
drivers/char/nwbutton.c | 2 +-
drivers/input/serio/i8042-io.h | 6 +++---
drivers/tty/serial/21285.c | 2 +-
15 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 09b2767fee0f..1155c78bb6aa 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -153,7 +153,7 @@ config ARM
select PCI_SYSCALL if PCI
select PERF_USE_VMALLOC
select RTC_LIB
- select SPARSE_IRQ if !ARCH_FOOTBRIDGE
+ select SPARSE_IRQ
select SYS_SUPPORTS_APM_EMULATION
select THREAD_INFO_IN_TASK
select TIMER_OF if OF
diff --git a/arch/arm/include/asm/irq.h b/arch/arm/include/asm/irq.h
index 26c1d2ced4ce..08589b88c3b9 100644
--- a/arch/arm/include/asm/irq.h
+++ b/arch/arm/include/asm/irq.h
@@ -10,7 +10,9 @@
#define NR_IRQS NR_IRQS_LEGACY
#endif
-#ifndef irq_canonicalize
+#ifdef CONFIG_ARCH_FOOTBRIDGE
+#define irq_canonicalize(i) (((i) == 2) ? 9 : i)
+#else
#define irq_canonicalize(i) (i)
#endif
--git a/arch/arm/mach-footbridge/dc21285-timer.c b/arch/arm/mach-footbridge/dc21285-timer.c
index 2908c9ef3c9b..7d7ad1c1ef3f 100644
--- a/arch/arm/mach-footbridge/dc21285-timer.c
+++ b/arch/arm/mach-footbridge/dc21285-timer.c
@@ -12,7 +12,7 @@
#include <linux/irq.h>
#include <linux/sched_clock.h>
-#include <asm/irq.h>
+#include <mach/irqs.h>
#include <asm/hardware/dec21285.h>
#include <asm/mach/time.h>
--git a/arch/arm/mach-footbridge/dc21285.c b/arch/arm/mach-footbridge/dc21285.c
index e1b336624883..ffdecfadc9e2 100644
--- a/arch/arm/mach-footbridge/dc21285.c
+++ b/arch/arm/mach-footbridge/dc21285.c
@@ -17,7 +17,7 @@
#include <linux/io.h>
#include <linux/spinlock.h>
-#include <asm/irq.h>
+#include <mach/irqs.h>
#include <asm/mach/pci.h>
#include <asm/hardware/dec21285.h>
--git a/arch/arm/mach-footbridge/ebsa285-pci.c b/arch/arm/mach-footbridge/ebsa285-pci.c
index c3f280d08fa7..d2168660dd01 100644
--- a/arch/arm/mach-footbridge/ebsa285-pci.c
+++ b/arch/arm/mach-footbridge/ebsa285-pci.c
@@ -10,7 +10,7 @@
#include <linux/pci.h>
#include <linux/init.h>
-#include <asm/irq.h>
+#include <mach/irqs.h>
#include <asm/mach/pci.h>
#include <asm/mach-types.h>
--git a/arch/arm/mach-footbridge/ebsa285.c b/arch/arm/mach-footbridge/ebsa285.c
index 1cb7d674bc81..a820f7467468 100644
--- a/arch/arm/mach-footbridge/ebsa285.c
+++ b/arch/arm/mach-footbridge/ebsa285.c
@@ -10,6 +10,7 @@
#include <linux/slab.h>
#include <linux/leds.h>
+#include <mach/irqs.h>
#include <asm/hardware/dec21285.h>
#include <asm/mach-types.h>
@@ -117,6 +118,7 @@ MACHINE_START(EBSA285, "EBSA285")
.video_end = 0x000bffff,
.map_io = footbridge_map_io,
.init_early = footbridge_sched_clock,
+ .nr_irqs = FOOTBRIDGE_NR_IRQS,
.init_irq = footbridge_init_irq,
.init_time = footbridge_timer_init,
.restart = footbridge_restart,
--git a/arch/arm/mach-footbridge/include/mach/irqs.h b/arch/arm/mach-footbridge/include/mach/irqs.h
index a5f41846ab9c..10f1fbc24012 100644
--- a/arch/arm/mach-footbridge/include/mach/irqs.h
+++ b/arch/arm/mach-footbridge/include/mach/irqs.h
@@ -11,7 +11,7 @@
*/
#include <asm/mach-types.h>
-#define NR_IRQS 36
+#define FOOTBRIDGE_NR_IRQS 36
#define NR_DC21285_IRQS 16
#define _ISA_IRQ(x) (0 + (x))
@@ -93,5 +93,3 @@
#define I8042_KBD_IRQ IRQ_ISA_KEYBOARD
#define I8042_AUX_IRQ (machine_is_netwinder() ? IRQ_NETWINDER_PS2MOUSE : IRQ_ISA_PS2MOUSE)
#define IRQ_FLOPPYDISK IRQ_ISA_FLOPPY
-
-#define irq_canonicalize(_i) (((_i) == IRQ_ISA_CASCADE) ? IRQ_ISA_2 : _i)
--git a/arch/arm/mach-footbridge/isa-irq.c b/arch/arm/mach-footbridge/isa-irq.c
index 842ddb4121ef..e4e71bdf1dc7 100644
--- a/arch/arm/mach-footbridge/isa-irq.c
+++ b/arch/arm/mach-footbridge/isa-irq.c
@@ -21,8 +21,8 @@
#include <asm/mach/irq.h>
#include <mach/hardware.h>
+#include <mach/irqs.h>
#include <asm/hardware/dec21285.h>
-#include <asm/irq.h>
#include <asm/mach-types.h>
#include "common.h"
--git a/arch/arm/mach-footbridge/isa-timer.c b/arch/arm/mach-footbridge/isa-timer.c
index 723e3eae995d..07dee61b0b03 100644
--- a/arch/arm/mach-footbridge/isa-timer.c
+++ b/arch/arm/mach-footbridge/isa-timer.c
@@ -13,7 +13,7 @@
#include <linux/spinlock.h>
#include <linux/timex.h>
-#include <asm/irq.h>
+#include <mach/irqs.h>
#include <asm/mach/time.h>
#include "common.h"
--git a/arch/arm/mach-footbridge/isa.c b/arch/arm/mach-footbridge/isa.c
index 84caccddce44..1e7b0f5fb111 100644
--- a/arch/arm/mach-footbridge/isa.c
+++ b/arch/arm/mach-footbridge/isa.c
@@ -7,7 +7,7 @@
#include <linux/init.h>
#include <linux/serial_8250.h>
-#include <asm/irq.h>
+#include <mach/irqs.h>
#include <asm/hardware/dec21285.h>
#include "common.h"
--git a/arch/arm/mach-footbridge/netwinder-hw.c b/arch/arm/mach-footbridge/netwinder-hw.c
index c024eefd4978..ab17ba916d47 100644
--- a/arch/arm/mach-footbridge/netwinder-hw.c
+++ b/arch/arm/mach-footbridge/netwinder-hw.c
@@ -16,6 +16,7 @@
#include <linux/slab.h>
#include <linux/leds.h>
+#include <mach/irqs.h>
#include <asm/hardware/dec21285.h>
#include <asm/mach-types.h>
#include <asm/setup.h>
@@ -766,6 +767,7 @@ MACHINE_START(NETWINDER, "Rebel-NetWinder")
.reserve_lp2 = 1,
.fixup = fixup_netwinder,
.map_io = footbridge_map_io,
+ .nr_irqs = FOOTBRIDGE_NR_IRQS,
.init_irq = footbridge_init_irq,
.init_time = isa_timer_init,
.restart = netwinder_restart,
--git a/arch/arm/mach-footbridge/netwinder-pci.c b/arch/arm/mach-footbridge/netwinder-pci.c
index e8304392074b..bfd5c0606c71 100644
--- a/arch/arm/mach-footbridge/netwinder-pci.c
+++ b/arch/arm/mach-footbridge/netwinder-pci.c
@@ -10,7 +10,7 @@
#include <linux/pci.h>
#include <linux/init.h>
-#include <asm/irq.h>
+#include <mach/irqs.h>
#include <asm/mach/pci.h>
#include <asm/mach-types.h>
diff --git a/drivers/char/nwbutton.c b/drivers/char/nwbutton.c
index 92cee5717237..99819b184aac 100644
--- a/drivers/char/nwbutton.c
+++ b/drivers/char/nwbutton.c
@@ -18,7 +18,7 @@
#include <linux/init.h>
#include <linux/uaccess.h>
-#include <asm/irq.h>
+#include <mach/irqs.h>
#include <asm/mach-types.h>
#define __NWBUTTON_C /* Tell the header file who we are */
diff --git a/drivers/input/serio/i8042-io.h b/drivers/input/serio/i8042-io.h
index a8f4b2d70e59..cea72bd888af 100644
--- a/drivers/input/serio/i8042-io.h
+++ b/drivers/input/serio/i8042-io.h
@@ -15,9 +15,9 @@
* IRQs.
*/
-#if defined(__arm__)
-/* defined in include/asm-arm/arch-xxx/irqs.h */
-#include <asm/irq.h>
+#ifdef CONFIG_ARCH_FOOTBRIDGE
+/* defined in arch/arm/mach-footbridge/include/mach/irqs.h */
+#include <mach/irqs.h>
#elif defined(CONFIG_PPC)
extern int of_i8042_kbd_irq;
extern int of_i8042_aux_irq;
diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c
index 4de0c975ebdc..f0c63875912e 100644
--- a/drivers/tty/serial/21285.c
+++ b/drivers/tty/serial/21285.c
@@ -15,11 +15,11 @@
#include <linux/serial.h>
#include <linux/io.h>
-#include <asm/irq.h>
#include <asm/mach-types.h>
#include <asm/system_info.h>
#include <asm/hardware/dec21285.h>
#include <mach/hardware.h>
+#include <mach/irqs.h>
#define BAUD_BASE (mem_fclk_21285/64)
--
2.43.0
^ permalink raw reply related
* [PATCH 6.12.y 0/8] serial: 8250_dw: backport BUSY deassert series
From: Ionut Nechita (Wind River) @ 2026-05-10 13:40 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh; +Cc: andriy.shevchenko, linux-serial, Ionut Nechita
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;
* 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.
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(-)
--
2.54.0
^ permalink raw reply
* [PATCH 6.12.y 2/8] serial: 8250_dw: Comment possible corner cases in serial_out() implementation
From: Ionut Nechita (Wind River) @ 2026-05-10 13:40 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh; +Cc: andriy.shevchenko, linux-serial, Ionut Nechita
In-Reply-To: <20260510134011.618215-1-ionut.nechita@windriver.com>
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
commit bd8cad85561b8de36f099404c332bf3e3dbe90f5 upstream.
8250 DesignWare driver uses a few custom implementations of the serial_out().
These implementations are carefully made to avoid infinite loops. But this is
not obvious from looking at the code. Comment the possible corner cases in
the respective functions.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20250317094021.1201512-1-andriy.shevchenko@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_dw.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 3225011fd772..88c55336d50f 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -107,11 +107,23 @@ static inline int dw8250_modify_msr(struct uart_port *p, int offset, int value)
return value;
}
+/*
+ * This function is being called as part of the uart_port::serial_out()
+ * routine. Hence, it must not call serial_port_out() or serial_out()
+ * against the modified registers here, i.e. LCR.
+ */
static void dw8250_force_idle(struct uart_port *p)
{
struct uart_8250_port *up = up_to_u8250p(p);
unsigned int lsr;
+ /*
+ * The following call currently performs serial_out()
+ * against the FCR register. Because it differs to LCR
+ * there will be no infinite loop, but if it ever gets
+ * modified, we might need a new custom version of it
+ * that avoids infinite recursion.
+ */
serial8250_clear_and_reinit_fifos(up);
/*
@@ -128,6 +140,11 @@ static void dw8250_force_idle(struct uart_port *p)
serial_port_in(p, UART_RX);
}
+/*
+ * This function is being called as part of the uart_port::serial_out()
+ * routine. Hence, it must not call serial_port_out() or serial_out()
+ * against the modified registers here, i.e. LCR.
+ */
static void dw8250_check_lcr(struct uart_port *p, int offset, int value)
{
struct dw8250_data *d = to_dw8250_data(p->private_data);
--
2.54.0
^ permalink raw reply related
* [PATCH 6.12.y 1/8] serial: 8250: use serial_port_in/out() helpers
From: Ionut Nechita (Wind River) @ 2026-05-10 13:40 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: andriy.shevchenko, linux-serial, Jiri Slaby (SUSE), Ionut Nechita
In-Reply-To: <20260510134011.618215-1-ionut.nechita@windriver.com>
From: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
commit dbd26a886e94deb7fda9050f9195ccb41f9a5d93 upstream.
There are serial_port_in/out() helpers to be used instead of direct
p->serial_in/out(). Use them in various 8250 drivers.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
[v2]
* Use serial_port_in/out() and not serial_in/out() [Andy]
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # 8250_dw
Link: https://lore.kernel.org/r/20250317070046.24386-28-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_dw.c | 16 ++++++++--------
drivers/tty/serial/8250/8250_fsl.c | 8 ++++----
drivers/tty/serial/8250/8250_omap.c | 2 +-
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index f17dc3de020c..3225011fd772 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -120,12 +120,12 @@ static void dw8250_force_idle(struct uart_port *p)
* enabled.
*/
if (up->fcr & UART_FCR_ENABLE_FIFO) {
- lsr = p->serial_in(p, UART_LSR);
+ lsr = serial_port_in(p, UART_LSR);
if (!(lsr & UART_LSR_DR))
return;
}
- (void)p->serial_in(p, UART_RX);
+ serial_port_in(p, UART_RX);
}
static void dw8250_check_lcr(struct uart_port *p, int offset, int value)
@@ -139,7 +139,7 @@ static void dw8250_check_lcr(struct uart_port *p, int offset, int value)
/* Make sure LCR write wasn't ignored */
while (tries--) {
- unsigned int lcr = p->serial_in(p, offset);
+ unsigned int lcr = serial_port_in(p, offset);
if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
return;
@@ -260,7 +260,7 @@ static int dw8250_handle_irq(struct uart_port *p)
{
struct uart_8250_port *up = up_to_u8250p(p);
struct dw8250_data *d = to_dw8250_data(p->private_data);
- unsigned int iir = p->serial_in(p, UART_IIR);
+ unsigned int iir = serial_port_in(p, UART_IIR);
bool rx_timeout = (iir & 0x3f) == UART_IIR_RX_TIMEOUT;
unsigned int quirks = d->pdata->quirks;
unsigned int status;
@@ -281,7 +281,7 @@ static int dw8250_handle_irq(struct uart_port *p)
status = serial_lsr_in(up);
if (!(status & (UART_LSR_DR | UART_LSR_BI)))
- (void) p->serial_in(p, UART_RX);
+ serial_port_in(p, UART_RX);
uart_port_unlock_irqrestore(p, flags);
}
@@ -303,7 +303,7 @@ static int dw8250_handle_irq(struct uart_port *p)
if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
/* Clear the USR */
- (void)p->serial_in(p, d->pdata->usr_reg);
+ serial_port_in(p, d->pdata->usr_reg);
return 1;
}
@@ -390,7 +390,7 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios,
static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
{
struct uart_8250_port *up = up_to_u8250p(p);
- unsigned int mcr = p->serial_in(p, UART_MCR);
+ unsigned int mcr = serial_port_in(p, UART_MCR);
if (up->capabilities & UART_CAP_IRDA) {
if (termios->c_line == N_IRDA)
@@ -398,7 +398,7 @@ static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
else
mcr &= ~DW_UART_MCR_SIRE;
- p->serial_out(p, UART_MCR, mcr);
+ serial_port_out(p, UART_MCR, mcr);
}
serial8250_do_set_ldisc(p, termios);
}
diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c
index b4ed442082a8..59d3d2801c2e 100644
--- a/drivers/tty/serial/8250/8250_fsl.c
+++ b/drivers/tty/serial/8250/8250_fsl.c
@@ -32,7 +32,7 @@ int fsl8250_handle_irq(struct uart_port *port)
uart_port_lock_irqsave(&up->port, &flags);
- iir = port->serial_in(port, UART_IIR);
+ iir = serial_port_in(port, UART_IIR);
if (iir & UART_IIR_NO_INT) {
uart_port_unlock_irqrestore(&up->port, flags);
return 0;
@@ -54,12 +54,12 @@ int fsl8250_handle_irq(struct uart_port *port)
if (unlikely((iir & UART_IIR_ID) == UART_IIR_RLSI &&
(up->lsr_saved_flags & UART_LSR_BI))) {
up->lsr_saved_flags &= ~UART_LSR_BI;
- port->serial_in(port, UART_RX);
+ serial_port_in(port, UART_RX);
uart_port_unlock_irqrestore(&up->port, flags);
return 1;
}
- lsr = orig_lsr = up->port.serial_in(&up->port, UART_LSR);
+ lsr = orig_lsr = serial_port_in(port, UART_LSR);
/* Process incoming characters first */
if ((lsr & (UART_LSR_DR | UART_LSR_BI)) &&
@@ -71,7 +71,7 @@ int fsl8250_handle_irq(struct uart_port *port)
if ((orig_lsr & UART_LSR_OE) && (up->overrun_backoff_time_ms > 0)) {
unsigned long delay;
- up->ier = port->serial_in(port, UART_IER);
+ up->ier = serial_port_in(port, UART_IER);
if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) {
port->ops->stop_rx(port);
} else {
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 0f4ce0c69114..dc9e3e25d55f 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -688,7 +688,7 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id)
/* Synchronize UART_IER access against the console. */
uart_port_lock(port);
- up->ier = port->serial_in(port, UART_IER);
+ up->ier = serial_port_in(port, UART_IER);
if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) {
port->ops->stop_rx(port);
} else {
--
2.54.0
^ permalink raw reply related
* [PATCH 6.12.y 6/8] serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling
From: Ionut Nechita (Wind River) @ 2026-05-10 13:40 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: andriy.shevchenko, linux-serial, Bandal, Shankar, Murthy, Shanth,
stable, Ionut Nechita
In-Reply-To: <20260510134011.618215-1-ionut.nechita@windriver.com>
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
commit 883c5a2bc934c165c4491d1ef7da0ac4e9765077 upstream.
dw8250_handle_irq() takes port's lock multiple times with no good
reason to release it in between and calls serial8250_handle_irq()
that also takes port's lock.
Take port's lock only once in dw8250_handle_irq() and use
serial8250_handle_irq_locked() to avoid releasing port's lock in
between.
As IIR_NO_INT check in serial8250_handle_irq() was outside of port's
lock, it has to be done already in dw8250_handle_irq().
DW UART can, in addition to IIR_NO_INT, report BUSY_DETECT (0x7) which
collided with the IIR_NO_INT (0x1) check in serial8250_handle_irq()
(because & is used instead of ==) meaning that no other work is done by
serial8250_handle_irq() during an BUSY_DETECT interrupt.
This allows reorganizing code in dw8250_handle_irq() to do both
IIR_NO_INT and BUSY_DETECT handling right at the start simplifying
the logic.
Tested-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Murthy, Shanth <shanth.murthy@intel.com>
Cc: stable <stable@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://patch.msgid.link/20260203171049.4353-5-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Ionut: adapt to 6.12.y - replace guard(uart_port_lock_irqsave)(p)
in dw8250_handle_irq() with explicit uart_port_lock_irqsave/
unlock_irqrestore around the post-switch body; the cleanup-based
uart_port_lock_irqsave guard class is not present in 6.12.y.]
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_dw.c | 36 +++++++++++++++++++------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 05e45b63e5f5..1076b72c120b 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -9,6 +9,9 @@
* LCR is written whilst busy. If it is, then a busy detect interrupt is
* raised, the LCR needs to be rewritten and the uart status register read.
*/
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/device.h>
@@ -40,6 +43,8 @@
#define RZN1_UART_RDMACR 0x110 /* DMA Control Register Receive Mode */
/* DesignWare specific register fields */
+#define DW_UART_IIR_IID GENMASK(3, 0)
+
#define DW_UART_MCR_SIRE BIT(6)
/* Renesas specific register fields */
@@ -314,6 +319,19 @@ static int dw8250_handle_irq(struct uart_port *p)
unsigned int status;
unsigned long flags;
+ switch (FIELD_GET(DW_UART_IIR_IID, iir)) {
+ case UART_IIR_NO_INT:
+ return 0;
+
+ case UART_IIR_BUSY:
+ /* Clear the USR */
+ serial_port_in(p, d->pdata->usr_reg);
+
+ return 1;
+ }
+
+ uart_port_lock_irqsave(p, &flags);
+
/*
* There are ways to get Designware-based UARTs into a state where
* they are asserting UART_IIR_RX_TIMEOUT but there is no actual
@@ -325,20 +343,15 @@ static int dw8250_handle_irq(struct uart_port *p)
* so we limit the workaround only to non-DMA mode.
*/
if (!up->dma && rx_timeout) {
- uart_port_lock_irqsave(p, &flags);
status = serial_lsr_in(up);
if (!(status & (UART_LSR_DR | UART_LSR_BI)))
serial_port_in(p, UART_RX);
-
- uart_port_unlock_irqrestore(p, flags);
}
/* Manually stop the Rx DMA transfer when acting as flow controller */
if (quirks & DW_UART_QUIRK_IS_DMA_FC && up->dma && up->dma->rx_running && rx_timeout) {
- uart_port_lock_irqsave(p, &flags);
status = serial_lsr_in(up);
- uart_port_unlock_irqrestore(p, flags);
if (status & (UART_LSR_DR | UART_LSR_BI)) {
dw8250_writel_ext(p, RZN1_UART_RDMACR, 0);
@@ -346,17 +359,11 @@ static int dw8250_handle_irq(struct uart_port *p)
}
}
- if (serial8250_handle_irq(p, iir))
- return 1;
-
- if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
- /* Clear the USR */
- serial_port_in(p, d->pdata->usr_reg);
+ serial8250_handle_irq_locked(p, iir);
- return 1;
- }
+ uart_port_unlock_irqrestore(p, flags);
- return 0;
+ return 1;
}
static void dw8250_clk_work_cb(struct work_struct *work)
@@ -859,6 +866,7 @@ static struct platform_driver dw8250_platform_driver = {
module_platform_driver(dw8250_platform_driver);
+MODULE_IMPORT_NS("SERIAL_8250");
MODULE_AUTHOR("Jamie Iles");
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Synopsys DesignWare 8250 serial port driver");
--
2.54.0
^ permalink raw reply related
* [PATCH 6.12.y 5/8] serial: 8250: Add serial8250_handle_irq_locked()
From: Ionut Nechita (Wind River) @ 2026-05-10 13:40 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: andriy.shevchenko, linux-serial, Bandal, Shankar, Murthy, Shanth,
stable, Ionut Nechita
In-Reply-To: <20260510134011.618215-1-ionut.nechita@windriver.com>
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
commit 8324a54f604da18f21070702a8ad82ab2062787b upstream.
8250_port exports serial8250_handle_irq() to HW specific 8250 drivers.
It takes port's lock within but a HW specific 8250 driver may want to
take port's lock itself, do something, and then call the generic
handler in 8250_port but to do that, the caller has to release port's
lock for no good reason.
Introduce serial8250_handle_irq_locked() which a HW specific driver can
call while already holding port's lock.
As this is new export, put it straight into a namespace (where all 8250
exports should eventually be moved).
Tested-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Murthy, Shanth <shanth.murthy@intel.com>
Cc: stable <stable@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://patch.msgid.link/20260203171049.4353-4-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Ionut: adapt to 6.12.y -
- replace guard(uart_port_lock_irqsave)(port) in
serial8250_handle_irq() with explicit uart_port_lock_irqsave/
unlock_irqrestore; the cleanup-based uart_port_lock_irqsave
guard class is not present in 6.12.y;
- use the bare-identifier form EXPORT_SYMBOL_NS_GPL(...,
SERIAL_8250) rather than the string form ("SERIAL_8250");
in 6.12.y the macro stringifies the namespace argument via
__stringify(ns), so it must be an unquoted identifier.]
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_port.c | 27 +++++++++++++++++++--------
include/linux/serial_8250.h | 1 +
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index d0f2e634ac30..1e6dca739eab 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -18,6 +18,7 @@
#include <linux/irq.h>
#include <linux/console.h>
#include <linux/gpio/consumer.h>
+#include <linux/lockdep.h>
#include <linux/sysrq.h>
#include <linux/delay.h>
#include <linux/platform_device.h>
@@ -1884,20 +1885,16 @@ static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
}
/*
- * This handles the interrupt from one port.
+ * Context: port's lock must be held by the caller.
*/
-int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
+void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
{
struct uart_8250_port *up = up_to_u8250p(port);
struct tty_port *tport = &port->state->port;
bool skip_rx = false;
- unsigned long flags;
u16 status;
- if (iir & UART_IIR_NO_INT)
- return 0;
-
- uart_port_lock_irqsave(port, &flags);
+ lockdep_assert_held_once(&port->lock);
status = serial_lsr_in(up);
@@ -1930,8 +1927,22 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
else if (!up->dma->tx_running)
__stop_tx(up);
}
+}
+EXPORT_SYMBOL_NS_GPL(serial8250_handle_irq_locked, SERIAL_8250);
- uart_unlock_and_check_sysrq_irqrestore(port, flags);
+/*
+ * This handles the interrupt from one port.
+ */
+int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
+{
+ unsigned long flags;
+
+ if (iir & UART_IIR_NO_INT)
+ return 0;
+
+ uart_port_lock_irqsave(port, &flags);
+ serial8250_handle_irq_locked(port, iir);
+ uart_port_unlock_irqrestore(port, flags);
return 1;
}
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index e0717c8393d7..7e0439ecd496 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -195,6 +195,7 @@ void serial8250_do_set_mctrl(struct uart_port *port, unsigned int mctrl);
void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
unsigned int quot);
int fsl8250_handle_irq(struct uart_port *port);
+void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir);
int serial8250_handle_irq(struct uart_port *port, unsigned int iir);
u16 serial8250_rx_chars(struct uart_8250_port *up, u16 lsr);
void serial8250_read_char(struct uart_8250_port *up, u16 lsr);
--
2.54.0
^ permalink raw reply related
* [PATCH 6.12.y 3/8] serial: 8250: Protect LCR write in shutdown
From: Ionut Nechita (Wind River) @ 2026-05-10 13:40 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: andriy.shevchenko, linux-serial, Bandal, Shankar, Murthy, Shanth,
stable, Ionut Nechita
In-Reply-To: <20260510134011.618215-1-ionut.nechita@windriver.com>
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
commit 59a33d83bbe6d73d2071d7ae21590b29faed0503 upstream.
The 8250_dw driver needs to potentially perform very complex operations
during LCR writes because its BUSY handling prevents updates to LCR
while UART is BUSY (which is not fully under our control without those
complex operations). Thus, LCR writes should occur under port's lock.
Move LCR write under port's lock in serial8250_do_shutdown(). Also
split the LCR RMW so that the logic is on a separate line for clarity.
Reported-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Murthy, Shanth <shanth.murthy@intel.com>
Cc: stable <stable@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://patch.msgid.link/20260203171049.4353-2-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Ionut: adapt to 6.12.y - keep both 'unsigned long flags' and the
new 'u32 lcr' declaration; place the LCR RMW inside the existing
explicit uart_port_lock_irqsave/unlock_irqrestore region rather
than the scoped_guard() block used upstream (cleanup-based
uart_port_lock_irqsave guard class is not present in 6.12.y).]
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_port.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index b4c8388ea6fc..d0f2e634ac30 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2462,6 +2462,7 @@ void serial8250_do_shutdown(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned long flags;
+ u32 lcr;
serial8250_rpm_get(up);
/*
@@ -2488,13 +2489,13 @@ void serial8250_do_shutdown(struct uart_port *port)
port->mctrl &= ~TIOCM_OUT2;
serial8250_set_mctrl(port, port->mctrl);
+
+ /* Disable break condition */
+ lcr = serial_port_in(port, UART_LCR);
+ lcr &= ~UART_LCR_SBC;
+ serial_port_out(port, UART_LCR, lcr);
uart_port_unlock_irqrestore(port, flags);
- /*
- * Disable break condition and FIFOs
- */
- serial_port_out(port, UART_LCR,
- serial_port_in(port, UART_LCR) & ~UART_LCR_SBC);
serial8250_clear_fifos(up);
#ifdef CONFIG_SERIAL_8250_RSA
--
2.54.0
^ permalink raw reply related
* [PATCH 6.12.y 7/8] serial: 8250_dw: Rework IIR_NO_INT handling to stop interrupt storm
From: Ionut Nechita (Wind River) @ 2026-05-10 13:40 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: andriy.shevchenko, linux-serial, stable, Bandal, Shankar,
Murthy, Shanth, Ionut Nechita
In-Reply-To: <20260510134011.618215-1-ionut.nechita@windriver.com>
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
commit 73a4ed8f9efaaaf8207614ccc1c9d5ca1888f23a upstream.
INTC10EE UART can end up into an interrupt storm where it reports
IIR_NO_INT (0x1). If the storm happens during active UART operation, it
is promptly stopped by IIR value change due to Rx or Tx events.
However, when there is no activity, either due to idle serial line or
due to specific circumstances such as during shutdown that writes
IER=0, there is nothing to stop the storm.
During shutdown the storm is particularly problematic because
serial8250_do_shutdown() calls synchronize_irq() that will hang in
waiting for the storm to finish which never happens.
This problem can also result in triggering a warning:
irq 45: nobody cared (try booting with the "irqpoll" option)
[...snip...]
handlers:
serial8250_interrupt
Disabling IRQ #45
Normal means to reset interrupt status by reading LSR, MSR, USR, or RX
register do not result in the UART deasserting the IRQ.
Add a quirk to INTC10EE UARTs to enable Tx interrupts if UART's Tx is
currently empty and inactive. Rework IIR_NO_INT to keep track of the
number of consecutive IIR_NO_INT, and on fourth one perform the quirk.
Enabling Tx interrupts should change IIR value from IIR_NO_INT to
IIR_THRI which has been observed to stop the storm.
Fixes: e92fad024929 ("serial: 8250_dw: Add ACPI ID for Granite Rapids-D UART")
Cc: stable <stable@kernel.org>
Reported-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Murthy, Shanth <shanth.murthy@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://patch.msgid.link/20260203171049.4353-6-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Ionut: adapt to 6.12.y -
- move uart_port_lock_irqsave() before the switch (matching the
upstream guard() placement) and convert the early returns into
a goto-out pattern with uart_port_unlock_irqrestore;
- drop the dw8250_setup_dma_filter() call (helper introduced by
c213375e3283 'serial: 8250_dw: Call dw8250_quirks() conditionally'
which is not backported);
- keep only the new p->shutdown = dw8250_shutdown assignment in the
existing 'else if (data->pdata)' branch; in 6.12.y p->handle_irq
is unconditionally set earlier in dw8250_probe() so the upstream
reassignment is unnecessary here.]
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_dw.c | 75 ++++++++++++++++++++++++++++---
1 file changed, 69 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 1076b72c120b..1650107d1cbc 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -61,6 +61,13 @@
#define DW_UART_QUIRK_IS_DMA_FC BIT(3)
#define DW_UART_QUIRK_APMC0D08 BIT(4)
#define DW_UART_QUIRK_CPR_VALUE BIT(5)
+#define DW_UART_QUIRK_IER_KICK BIT(6)
+
+/*
+ * Number of consecutive IIR_NO_INT interrupts required to trigger interrupt
+ * storm prevention code.
+ */
+#define DW_UART_QUIRK_IER_KICK_THRES 4
struct dw8250_platform_data {
u8 usr_reg;
@@ -82,6 +89,8 @@ struct dw8250_data {
unsigned int skip_autocfg:1;
unsigned int uart_16550_compatible:1;
+
+ u8 no_int_count;
};
static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
@@ -308,6 +317,29 @@ static unsigned int dw8250_serial_in32be(struct uart_port *p, int offset)
return dw8250_modify_msr(p, offset, value);
}
+/*
+ * INTC10EE UART can IRQ storm while reporting IIR_NO_INT. Inducing IIR value
+ * change has been observed to break the storm.
+ *
+ * If Tx is empty (THRE asserted), we use here IER_THRI to cause IIR_NO_INT ->
+ * IIR_THRI transition.
+ */
+static void dw8250_quirk_ier_kick(struct uart_port *p)
+{
+ struct uart_8250_port *up = up_to_u8250p(p);
+ u32 lsr;
+
+ if (up->ier & UART_IER_THRI)
+ return;
+
+ lsr = serial_lsr_in(up);
+ if (!(lsr & UART_LSR_THRE))
+ return;
+
+ serial_port_out(p, UART_IER, up->ier | UART_IER_THRI);
+ serial_port_in(p, UART_LCR); /* safe, no side-effects */
+ serial_port_out(p, UART_IER, up->ier);
+}
static int dw8250_handle_irq(struct uart_port *p)
{
@@ -318,19 +350,35 @@ static int dw8250_handle_irq(struct uart_port *p)
unsigned int quirks = d->pdata->quirks;
unsigned int status;
unsigned long flags;
+ int ret = 1;
+
+ uart_port_lock_irqsave(p, &flags);
switch (FIELD_GET(DW_UART_IIR_IID, iir)) {
case UART_IIR_NO_INT:
- return 0;
+ if (d->uart_16550_compatible || up->dma) {
+ ret = 0;
+ goto out;
+ }
+
+ if (quirks & DW_UART_QUIRK_IER_KICK &&
+ d->no_int_count == (DW_UART_QUIRK_IER_KICK_THRES - 1))
+ dw8250_quirk_ier_kick(p);
+ d->no_int_count = (d->no_int_count + 1) % DW_UART_QUIRK_IER_KICK_THRES;
+
+ ret = 0;
+ goto out;
case UART_IIR_BUSY:
/* Clear the USR */
serial_port_in(p, d->pdata->usr_reg);
- return 1;
+ d->no_int_count = 0;
+
+ goto out;
}
- uart_port_lock_irqsave(p, &flags);
+ d->no_int_count = 0;
/*
* There are ways to get Designware-based UARTs into a state where
@@ -361,9 +409,9 @@ static int dw8250_handle_irq(struct uart_port *p)
serial8250_handle_irq_locked(p, iir);
+out:
uart_port_unlock_irqrestore(p, flags);
-
- return 1;
+ return ret;
}
static void dw8250_clk_work_cb(struct work_struct *work)
@@ -561,6 +609,14 @@ static void dw8250_reset_control_assert(void *data)
reset_control_assert(data);
}
+static void dw8250_shutdown(struct uart_port *port)
+{
+ struct dw8250_data *d = to_dw8250_data(port->private_data);
+
+ serial8250_do_shutdown(port);
+ d->no_int_count = 0;
+}
+
static int dw8250_probe(struct platform_device *pdev)
{
struct uart_8250_port uart = {}, *up = &uart;
@@ -688,6 +744,8 @@ static int dw8250_probe(struct platform_device *pdev)
/* If the Busy Functionality is not implemented, don't handle it */
if (data->uart_16550_compatible)
p->handle_irq = NULL;
+ else if (data->pdata)
+ p->shutdown = dw8250_shutdown;
if (!data->skip_autocfg)
dw8250_setup_port(p);
@@ -819,6 +877,11 @@ static const struct dw8250_platform_data dw8250_skip_set_rate_data = {
.quirks = DW_UART_QUIRK_SKIP_SET_RATE,
};
+static const struct dw8250_platform_data dw8250_intc10ee = {
+ .usr_reg = DW_UART_USR,
+ .quirks = DW_UART_QUIRK_IER_KICK,
+};
+
static const struct of_device_id dw8250_of_match[] = {
{ .compatible = "snps,dw-apb-uart", .data = &dw8250_dw_apb },
{ .compatible = "cavium,octeon-3860-uart", .data = &dw8250_octeon_3860_data },
@@ -848,7 +911,7 @@ static const struct acpi_device_id dw8250_acpi_match[] = {
{ "INT33C5", (kernel_ulong_t)&dw8250_dw_apb },
{ "INT3434", (kernel_ulong_t)&dw8250_dw_apb },
{ "INT3435", (kernel_ulong_t)&dw8250_dw_apb },
- { "INTC10EE", (kernel_ulong_t)&dw8250_dw_apb },
+ { "INTC10EE", (kernel_ulong_t)&dw8250_intc10ee },
{ },
};
MODULE_DEVICE_TABLE(acpi, dw8250_acpi_match);
--
2.54.0
^ permalink raw reply related
* [PATCH 6.12.y 4/8] serial: 8250_dw: Avoid unnecessary LCR writes
From: Ionut Nechita (Wind River) @ 2026-05-10 13:40 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: andriy.shevchenko, linux-serial, Bandal, Shankar, Murthy, Shanth,
stable, Ionut Nechita
In-Reply-To: <20260510134011.618215-1-ionut.nechita@windriver.com>
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
commit 8002d6d6d0d8a36a7d6ca523b17a51cb0fa7c3c3 upstream.
When DW UART is configured with BUSY flag, LCR writes may not always
succeed which can make any LCR write complex and very expensive.
Performing write directly can trigger IRQ and the driver has to perform
complex and distruptive sequence while retrying the write.
Therefore, it's better to avoid doing LCR write that would not change
the value of the LCR register. Add LCR write avoidance code into the
8250_dw driver's .serial_out() functions.
Reported-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Murthy, Shanth <shanth.murthy@intel.com>
Cc: stable <stable@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://patch.msgid.link/20260203171049.4353-3-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_dw.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 88c55336d50f..05e45b63e5f5 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -181,6 +181,22 @@ static void dw8250_check_lcr(struct uart_port *p, int offset, int value)
*/
}
+/*
+ * With BUSY, LCR writes can be very expensive (IRQ + complex retry logic).
+ * If the write does not change the value of the LCR register, skip it entirely.
+ */
+static bool dw8250_can_skip_reg_write(struct uart_port *p, unsigned int offset, u32 value)
+{
+ struct dw8250_data *d = to_dw8250_data(p->private_data);
+ u32 lcr;
+
+ if (offset != UART_LCR || d->uart_16550_compatible)
+ return false;
+
+ lcr = serial_port_in(p, offset);
+ return lcr == value;
+}
+
/* Returns once the transmitter is empty or we run out of retries */
static void dw8250_tx_wait_empty(struct uart_port *p)
{
@@ -207,12 +223,18 @@ static void dw8250_tx_wait_empty(struct uart_port *p)
static void dw8250_serial_out(struct uart_port *p, int offset, int value)
{
+ if (dw8250_can_skip_reg_write(p, offset, value))
+ return;
+
writeb(value, p->membase + (offset << p->regshift));
dw8250_check_lcr(p, offset, value);
}
static void dw8250_serial_out38x(struct uart_port *p, int offset, int value)
{
+ if (dw8250_can_skip_reg_write(p, offset, value))
+ return;
+
/* Allow the TX to drain before we reconfigure */
if (offset == UART_LCR)
dw8250_tx_wait_empty(p);
@@ -237,6 +259,9 @@ static unsigned int dw8250_serial_inq(struct uart_port *p, int offset)
static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
{
+ if (dw8250_can_skip_reg_write(p, offset, value))
+ return;
+
value &= 0xff;
__raw_writeq(value, p->membase + (offset << p->regshift));
/* Read back to ensure register write ordering. */
@@ -248,6 +273,9 @@ static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
{
+ if (dw8250_can_skip_reg_write(p, offset, value))
+ return;
+
writel(value, p->membase + (offset << p->regshift));
dw8250_check_lcr(p, offset, value);
}
@@ -261,6 +289,9 @@ static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)
static void dw8250_serial_out32be(struct uart_port *p, int offset, int value)
{
+ if (dw8250_can_skip_reg_write(p, offset, value))
+ return;
+
iowrite32be(value, p->membase + (offset << p->regshift));
dw8250_check_lcr(p, offset, value);
}
--
2.54.0
^ permalink raw reply related
* [PATCH 6.12.y 8/8] serial: 8250_dw: Ensure BUSY is deasserted
From: Ionut Nechita (Wind River) @ 2026-05-10 13:40 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: andriy.shevchenko, linux-serial, stable, qianfan Zhao,
Adriana Nicolae, Bandal, Shankar, Murthy, Shanth, Ionut Nechita
In-Reply-To: <20260510134011.618215-1-ionut.nechita@windriver.com>
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
commit a7b9ce39fbe4ae2919fe4f7ac16c293cb6632d30 upstream.
DW UART cannot write to LCR, DLL, and DLH while BUSY is asserted.
Existance of BUSY depends on uart_16550_compatible, if UART HW is
configured with it those registers can always be written.
There currently is dw8250_force_idle() which attempts to achieve
non-BUSY state by disabling FIFO, however, the solution is unreliable
when Rx keeps getting more and more characters.
Create a sequence of operations that ensures UART cannot keep BUSY
asserted indefinitely. The new sequence relies on enabling loopback mode
temporarily to prevent incoming Rx characters keeping UART BUSY.
Ensure no Tx in ongoing while the UART is switches into the loopback
mode (requires exporting serial8250_fifo_wait_for_lsr_thre() and adding
DMA Tx pause/resume functions).
According to tests performed by Adriana Nicolae <adriana@arista.com>,
simply disabling FIFO or clearing FIFOs only once does not always
ensure BUSY is deasserted but up to two tries may be needed. This could
be related to ongoing Rx of a character (a guess, not known for sure).
Therefore, retry FIFO clearing a few times (retry limit 4 is arbitrary
number but using, e.g., p->fifosize seems overly large). Tests
performed by others did not exhibit similar challenge but it does not
seem harmful to leave the FIFO clearing loop in place for all DW UARTs
with BUSY functionality.
Use the new dw8250_idle_enter/exit() to do divisor writes and LCR
writes. In case of plain LCR writes, opportunistically try to update
LCR first and only invoke dw8250_idle_enter() if the write did not
succeed (it has been observed that in practice most LCR writes do
succeed without complications).
This issue was first reported by qianfan Zhao who put lots of debugging
effort into understanding the solution space.
Fixes: c49436b657d0 ("serial: 8250_dw: Improve unwritable LCR workaround")
Fixes: 7d4008ebb1c9 ("tty: add a DesignWare 8250 driver")
Cc: stable <stable@kernel.org>
Reported-by: qianfan Zhao <qianfanguijin@163.com>
Link: https://lore.kernel.org/linux-serial/289bb78a-7509-1c5c-2923-a04ed3b6487d@163.com/
Reported-by: Adriana Nicolae <adriana@arista.com>
Link: https://lore.kernel.org/linux-serial/20250819182322.3451959-1-adriana@arista.com/
Reported-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Murthy, Shanth <shanth.murthy@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://patch.msgid.link/20260203171049.4353-8-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Ionut: adapt to 6.12.y -
- trivial conflict in dw8250_check_lcr(): 6.12.y still has the
(int offset, int value) signature (the 'unsigned int offset,
u32 value' rework from fc9ceb501e38 is not backported), but
the conflicting tries/loop body is removed by this patch
anyway - took the new code as-is;
- use the bare-identifier form EXPORT_SYMBOL_NS_GPL(...,
SERIAL_8250) and MODULE_IMPORT_NS(SERIAL_8250) rather than
the string forms; in 6.12.y both macros stringify the
namespace argument via __stringify(ns), so it must be an
unquoted identifier.]
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250.h | 25 +++++
drivers/tty/serial/8250/8250_dw.c | 165 ++++++++++++++++++++--------
drivers/tty/serial/8250/8250_port.c | 28 ++---
3 files changed, 162 insertions(+), 56 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 10a706fe4b24..1fe8222ffacd 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -184,7 +184,9 @@ static unsigned int __maybe_unused serial_icr_read(struct uart_8250_port *up,
return value;
}
+void serial8250_clear_fifos(struct uart_8250_port *p);
void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p);
+void serial8250_fifo_wait_for_lsr_thre(struct uart_8250_port *up, unsigned int count);
static inline u32 serial_dl_read(struct uart_8250_port *up)
{
@@ -402,6 +404,26 @@ static inline bool serial8250_tx_dma_running(struct uart_8250_port *p)
return dma && dma->tx_running;
}
+
+static inline void serial8250_tx_dma_pause(struct uart_8250_port *p)
+{
+ struct uart_8250_dma *dma = p->dma;
+
+ if (!dma->tx_running)
+ return;
+
+ dmaengine_pause(dma->txchan);
+}
+
+static inline void serial8250_tx_dma_resume(struct uart_8250_port *p)
+{
+ struct uart_8250_dma *dma = p->dma;
+
+ if (!dma->tx_running)
+ return;
+
+ dmaengine_resume(dma->txchan);
+}
#else
static inline int serial8250_tx_dma(struct uart_8250_port *p)
{
@@ -423,6 +445,9 @@ static inline bool serial8250_tx_dma_running(struct uart_8250_port *p)
{
return false;
}
+
+static inline void serial8250_tx_dma_pause(struct uart_8250_port *p) { }
+static inline void serial8250_tx_dma_resume(struct uart_8250_port *p) { }
#endif
static inline int ns16550a_goto_highspeed(struct uart_8250_port *up)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 1650107d1cbc..630cc7227494 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -16,6 +16,7 @@
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/io.h>
+#include <linux/lockdep.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/notifier.h>
@@ -47,6 +48,8 @@
#define DW_UART_MCR_SIRE BIT(6)
+#define DW_UART_USR_BUSY BIT(0)
+
/* Renesas specific register fields */
#define RZN1_UART_xDMACR_DMA_EN BIT(0)
#define RZN1_UART_xDMACR_1_WORD_BURST (0 << 1)
@@ -89,6 +92,7 @@ struct dw8250_data {
unsigned int skip_autocfg:1;
unsigned int uart_16550_compatible:1;
+ unsigned int in_idle:1;
u8 no_int_count;
};
@@ -121,78 +125,151 @@ static inline int dw8250_modify_msr(struct uart_port *p, int offset, int value)
return value;
}
+static void dw8250_idle_exit(struct uart_port *p)
+{
+ struct dw8250_data *d = to_dw8250_data(p->private_data);
+ struct uart_8250_port *up = up_to_u8250p(p);
+
+ if (d->uart_16550_compatible)
+ return;
+
+ if (up->capabilities & UART_CAP_FIFO)
+ serial_port_out(p, UART_FCR, up->fcr);
+ serial_port_out(p, UART_MCR, up->mcr);
+ serial_port_out(p, UART_IER, up->ier);
+
+ /* DMA Rx is restarted by IRQ handler as needed. */
+ if (up->dma)
+ serial8250_tx_dma_resume(up);
+
+ d->in_idle = 0;
+}
+
/*
- * This function is being called as part of the uart_port::serial_out()
- * routine. Hence, it must not call serial_port_out() or serial_out()
- * against the modified registers here, i.e. LCR.
+ * Ensure BUSY is not asserted. If DW UART is configured with
+ * !uart_16550_compatible, the writes to LCR, DLL, and DLH fail while
+ * BUSY is asserted.
+ *
+ * Context: port's lock must be held
*/
-static void dw8250_force_idle(struct uart_port *p)
+static int dw8250_idle_enter(struct uart_port *p)
{
+ struct dw8250_data *d = to_dw8250_data(p->private_data);
+ unsigned int usr_reg = d->pdata ? d->pdata->usr_reg : DW_UART_USR;
struct uart_8250_port *up = up_to_u8250p(p);
- unsigned int lsr;
+ int retries;
+ u32 lsr;
- /*
- * The following call currently performs serial_out()
- * against the FCR register. Because it differs to LCR
- * there will be no infinite loop, but if it ever gets
- * modified, we might need a new custom version of it
- * that avoids infinite recursion.
- */
- serial8250_clear_and_reinit_fifos(up);
+ lockdep_assert_held_once(&p->lock);
+
+ if (d->uart_16550_compatible)
+ return 0;
+
+ d->in_idle = 1;
+
+ /* Prevent triggering interrupt from RBR filling */
+ serial_port_out(p, UART_IER, 0);
+
+ if (up->dma) {
+ serial8250_rx_dma_flush(up);
+ if (serial8250_tx_dma_running(up))
+ serial8250_tx_dma_pause(up);
+ }
/*
- * With PSLVERR_RESP_EN parameter set to 1, the device generates an
- * error response when an attempt to read an empty RBR with FIFO
- * enabled.
+ * Wait until Tx becomes empty + one extra frame time to ensure all bits
+ * have been sent on the wire.
+ *
+ * FIXME: frame_time delay is too long with very low baudrates.
*/
- if (up->fcr & UART_FCR_ENABLE_FIFO) {
- lsr = serial_port_in(p, UART_LSR);
- if (!(lsr & UART_LSR_DR))
- return;
+ serial8250_fifo_wait_for_lsr_thre(up, p->fifosize);
+ ndelay(p->frame_time);
+
+ serial_port_out(p, UART_MCR, up->mcr | UART_MCR_LOOP);
+
+ retries = 4; /* Arbitrary limit, 2 was always enough in tests */
+ do {
+ serial8250_clear_fifos(up);
+ if (!(serial_port_in(p, usr_reg) & DW_UART_USR_BUSY))
+ break;
+ /* FIXME: frame_time delay is too long with very low baudrates. */
+ ndelay(p->frame_time);
+ } while (--retries);
+
+ lsr = serial_lsr_in(up);
+ if (lsr & UART_LSR_DR) {
+ serial_port_in(p, UART_RX);
+ up->lsr_saved_flags = 0;
+ }
+
+ /* Now guaranteed to have BUSY deasserted? Just sanity check */
+ if (serial_port_in(p, usr_reg) & DW_UART_USR_BUSY) {
+ dw8250_idle_exit(p);
+ return -EBUSY;
}
- serial_port_in(p, UART_RX);
+ return 0;
+}
+
+static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
+ unsigned int quot, unsigned int quot_frac)
+{
+ struct uart_8250_port *up = up_to_u8250p(p);
+ int ret;
+
+ ret = dw8250_idle_enter(p);
+ if (ret < 0)
+ return;
+
+ serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
+ if (!(serial_port_in(p, UART_LCR) & UART_LCR_DLAB))
+ goto idle_failed;
+
+ serial_dl_write(up, quot);
+ serial_port_out(p, UART_LCR, up->lcr);
+
+idle_failed:
+ dw8250_idle_exit(p);
}
/*
* This function is being called as part of the uart_port::serial_out()
- * routine. Hence, it must not call serial_port_out() or serial_out()
- * against the modified registers here, i.e. LCR.
+ * routine. Hence, special care must be taken when serial_port_out() or
+ * serial_out() against the modified registers here, i.e. LCR (d->in_idle is
+ * used to break recursion loop).
*/
static void dw8250_check_lcr(struct uart_port *p, int offset, int value)
{
struct dw8250_data *d = to_dw8250_data(p->private_data);
- void __iomem *addr = p->membase + (offset << p->regshift);
- int tries = 1000;
+ u32 lcr;
+ int ret;
if (offset != UART_LCR || d->uart_16550_compatible)
return;
+ lcr = serial_port_in(p, UART_LCR);
+
/* Make sure LCR write wasn't ignored */
- while (tries--) {
- unsigned int lcr = serial_port_in(p, offset);
+ if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
+ return;
- if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
- return;
+ if (d->in_idle)
+ goto write_err;
- dw8250_force_idle(p);
+ ret = dw8250_idle_enter(p);
+ if (ret < 0)
+ goto write_err;
-#ifdef CONFIG_64BIT
- if (p->type == PORT_OCTEON)
- __raw_writeq(value & 0xff, addr);
- else
-#endif
- if (p->iotype == UPIO_MEM32)
- writel(value, addr);
- else if (p->iotype == UPIO_MEM32BE)
- iowrite32be(value, addr);
- else
- writeb(value, addr);
- }
+ serial_port_out(p, UART_LCR, value);
+ dw8250_idle_exit(p);
+ return;
+
+write_err:
/*
* FIXME: this deadlocks if port->lock is already held
* dev_err(p->dev, "Couldn't set LCR to %d\n", value);
*/
+ return; /* Silences "label at the end of compound statement" */
}
/*
@@ -636,8 +713,10 @@ static int dw8250_probe(struct platform_device *pdev)
p->type = PORT_8250;
p->flags = UPF_FIXED_PORT;
p->dev = dev;
+
p->set_ldisc = dw8250_set_ldisc;
p->set_termios = dw8250_set_termios;
+ p->set_divisor = dw8250_set_divisor;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
@@ -929,7 +1008,7 @@ static struct platform_driver dw8250_platform_driver = {
module_platform_driver(dw8250_platform_driver);
-MODULE_IMPORT_NS("SERIAL_8250");
+MODULE_IMPORT_NS(SERIAL_8250);
MODULE_AUTHOR("Jamie Iles");
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Synopsys DesignWare 8250 serial port driver");
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 1e6dca739eab..ce26d1649ca5 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -478,7 +478,7 @@ serial_port_out_sync(struct uart_port *p, int offset, int value)
/*
* FIFO support.
*/
-static void serial8250_clear_fifos(struct uart_8250_port *p)
+void serial8250_clear_fifos(struct uart_8250_port *p)
{
if (p->capabilities & UART_CAP_FIFO) {
serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO);
@@ -487,6 +487,7 @@ static void serial8250_clear_fifos(struct uart_8250_port *p)
serial_out(p, UART_FCR, 0);
}
}
+EXPORT_SYMBOL_NS_GPL(serial8250_clear_fifos, SERIAL_8250);
static enum hrtimer_restart serial8250_em485_handle_start_tx(struct hrtimer *t);
static enum hrtimer_restart serial8250_em485_handle_stop_tx(struct hrtimer *t);
@@ -3288,6 +3289,17 @@ void serial8250_set_defaults(struct uart_8250_port *up)
}
EXPORT_SYMBOL_GPL(serial8250_set_defaults);
+void serial8250_fifo_wait_for_lsr_thre(struct uart_8250_port *up, unsigned int count)
+{
+ unsigned int i;
+
+ for (i = 0; i < count; i++) {
+ if (wait_for_lsr(up, UART_LSR_THRE))
+ return;
+ }
+}
+EXPORT_SYMBOL_NS_GPL(serial8250_fifo_wait_for_lsr_thre, SERIAL_8250);
+
#ifdef CONFIG_SERIAL_8250_CONSOLE
static void serial8250_console_putchar(struct uart_port *port, unsigned char ch)
@@ -3324,16 +3336,6 @@ static void serial8250_console_restore(struct uart_8250_port *up)
serial8250_out_MCR(up, up->mcr | UART_MCR_DTR | UART_MCR_RTS);
}
-static void fifo_wait_for_lsr(struct uart_8250_port *up, unsigned int count)
-{
- unsigned int i;
-
- for (i = 0; i < count; i++) {
- if (wait_for_lsr(up, UART_LSR_THRE))
- return;
- }
-}
-
/*
* Print a string to the serial port using the device FIFO
*
@@ -3351,7 +3353,7 @@ static void serial8250_console_fifo_write(struct uart_8250_port *up,
while (s != end) {
/* Allow timeout for each byte of a possibly full FIFO */
- fifo_wait_for_lsr(up, fifosize);
+ serial8250_fifo_wait_for_lsr_thre(up, fifosize);
for (i = 0; i < fifosize && s != end; ++i) {
if (*s == '\n' && !cr_sent) {
@@ -3369,7 +3371,7 @@ static void serial8250_console_fifo_write(struct uart_8250_port *up,
* Allow timeout for each byte written since the caller will only wait
* for UART_LSR_BOTH_EMPTY using the timeout of a single character
*/
- fifo_wait_for_lsr(up, tx_count);
+ serial8250_fifo_wait_for_lsr_thre(up, tx_count);
}
/*
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] vt: keyboard: serialize KDGETLED with keyboard_tasklet
From: Greg KH @ 2026-05-10 16:21 UTC (permalink / raw)
To: Cen Zhang; +Cc: jirislaby, linux-kernel, linux-serial, baijiaju1990
In-Reply-To: <20260510031348.1922157-1-zzzccc427@gmail.com>
On Sun, May 10, 2026 at 11:13:48AM +0800, Cen Zhang wrote:
> KDGETLED reaches getledstate() and samples ledstate without serializing
> against keyboard_tasklet.
>
> That is problematic because kbd_bh() temporarily stores ~leds into
> ledstate during a VT switch to force LED propagation before committing
> the final value. An unlocked KDGETLED can therefore observe that
> internal sentinel instead of the last committed LED state.
Why is that an issue?
> Other ledstate readers in this file already quiesce keyboard_tasklet
> before sampling the value. Do the same in getledstate() so the ioctl
> only returns a stable snapshot.
stable for "what" exactly? This is a snapshot in time, be it before or
after it changes is not always going to really matter here, as it can
change right after you "enable" the tasklet, right?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] tty: n_tty: read termios under lock in poll
From: Greg KH @ 2026-05-10 16:23 UTC (permalink / raw)
To: Cen Zhang; +Cc: jirislaby, linux-kernel, linux-serial, baijiaju1990
In-Reply-To: <20260510025940.1884932-1-zzzccc427@gmail.com>
On Sun, May 10, 2026 at 10:59:40AM +0800, Cen Zhang wrote:
> n_tty_poll() uses input_available_p() to decide whether buffered input
> makes the tty readable. That helper reads termios state through
> L_EXTPROC(), VMIN, and VTIME, but the poll path does not hold the read
> side of tty->termios_rwsem.
>
> tty_set_termios() updates tty->termios under the write side of the same
> semaphore, including c_lflag and c_cc[]. n_tty_read() already takes the
> read side before reading the same termios fields and before calling
> input_available_p(). Protect the poll-side readiness checks the same way
> so poll observes a coherent termios state when deciding whether to report
> readable input.
But why does that matter? If it changes right after you grab/release
the lock, the data will be stale as well. What userspace logic is
broken because of there not being a lock held here?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] vt: keyboard: serialize KDGETLED with keyboard_tasklet
From: Cen Zhang @ 2026-05-10 16:43 UTC (permalink / raw)
To: Greg KH; +Cc: jirislaby, linux-kernel, linux-serial, baijiaju1990
In-Reply-To: <2026051001-peso-enamel-2e62@gregkh>
Hi Greg,
> On Sun, May 10, 2026 at 06:22:xxPM +0200, Greg KH wrote:
> Why is that an issue?
Thanks, that's a fair point. The issue I was trying to describe is not
that KDGETLED can race with a normal LED state transition and return
either the old or the new value. That part is just the usual snapshot
semantics, and the value can of course change immediately after the ioctl
returns.
The specific case I was concerned about is the VT-switch force-update
path. vt_set_leds_compute_shiftstate() sets vt_switch and schedules
keyboard_tasklet, and then kbd_bh() does:
if (vt_switch) {
ledstate = ~leds;
vt_switch = false;
}
if (leds != ledstate) {
kbd_propagate_led_state(ledstate, leds);
ledstate = leds;
}
That first assignment is only an internal old-state sentinel to force LED
propagation when switching VTs. It is not a committed LED state intended
to be returned to userspace. Since KDGETLED currently returns
ledstate & 0xff without quiescing the tasklet, it can expose that
temporary complemented value. For example, if the real LED state is
LED_NUM only, a read in that window can return the complemented low byte
instead of 0x02.
> stable for "what" exactly? This is a snapshot in time, be it before or
> after it changes is not always going to really matter here, as it can
> change right after you "enable" the tasklet, right?
Yes, I agree. "stable snapshot" was too broad a description.
By "stable" I only meant "not while kbd_bh() is in the middle of using
that internal force-update value". tasklet_disable() would make KDGETLED
observe either the old value before the tasklet runs, or the committed
value after it finishes. It does not make the value stable after
tasklet_enable(), and the changelog should not imply that.
This is a small userspace-visible consistency issue, not a memory-safety
problem. If this behavior is considered acceptable for KDGETLED, I can
drop the patch; otherwise I can send a v2 with a narrower changelog.
Best regards,
Cen
^ permalink raw reply
* Re: [PATCH] vt: keyboard: serialize KDGETLED with keyboard_tasklet
From: Greg KH @ 2026-05-10 16:55 UTC (permalink / raw)
To: Cen Zhang; +Cc: jirislaby, linux-kernel, linux-serial, baijiaju1990
In-Reply-To: <CAFRLqsUeat=rUj7aCSAku=A9NBNj3Wxd+xi8xzzBtRWqCxp18Q@mail.gmail.com>
On Mon, May 11, 2026 at 12:43:26AM +0800, Cen Zhang wrote:
> Hi Greg,
>
> > On Sun, May 10, 2026 at 06:22:xxPM +0200, Greg KH wrote:
>
> > Why is that an issue?
>
> Thanks, that's a fair point. The issue I was trying to describe is not
> that KDGETLED can race with a normal LED state transition and return
> either the old or the new value. That part is just the usual snapshot
> semantics, and the value can of course change immediately after the ioctl
> returns.
>
> The specific case I was concerned about is the VT-switch force-update
> path. vt_set_leds_compute_shiftstate() sets vt_switch and schedules
> keyboard_tasklet, and then kbd_bh() does:
>
> if (vt_switch) {
> ledstate = ~leds;
> vt_switch = false;
> }
>
> if (leds != ledstate) {
> kbd_propagate_led_state(ledstate, leds);
> ledstate = leds;
> }
>
> That first assignment is only an internal old-state sentinel to force LED
> propagation when switching VTs. It is not a committed LED state intended
> to be returned to userspace. Since KDGETLED currently returns
> ledstate & 0xff without quiescing the tasklet, it can expose that
> temporary complemented value. For example, if the real LED state is
> LED_NUM only, a read in that window can return the complemented low byte
> instead of 0x02.
>
> > stable for "what" exactly? This is a snapshot in time, be it before or
> > after it changes is not always going to really matter here, as it can
> > change right after you "enable" the tasklet, right?
>
> Yes, I agree. "stable snapshot" was too broad a description.
>
> By "stable" I only meant "not while kbd_bh() is in the middle of using
> that internal force-update value". tasklet_disable() would make KDGETLED
> observe either the old value before the tasklet runs, or the committed
> value after it finishes. It does not make the value stable after
> tasklet_enable(), and the changelog should not imply that.
>
> This is a small userspace-visible consistency issue, not a memory-safety
> problem. If this behavior is considered acceptable for KDGETLED, I can
> drop the patch; otherwise I can send a v2 with a narrower changelog.
In the past decades of use, has anyone every noticed or complained about
this consitency issue? If not, I would recommend not worrying about it :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] vt: keyboard: serialize KDGETLED with keyboard_tasklet
From: Cen Zhang @ 2026-05-10 17:01 UTC (permalink / raw)
To: Greg KH; +Cc: jirislaby, linux-kernel, linux-serial, baijiaju1990
In-Reply-To: <2026051039-snitch-attitude-a67d@gregkh>
Hi Greg,
> In the past decades of use, has anyone every noticed or complained about
> this consitency issue? If not, I would recommend not worrying about it :)
>
Understood, thanks. I will drop this patch.
Thank you for the review and guidance.
Best regards,
Cen
^ permalink raw reply
* Re: [PATCH v7 6/6] ARM: zte: defconfig: Add a zx29 defconfig file
From: Linus Walleij @ 2026-05-11 9:00 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Jonathan Corbet, Shuah Khan, Russell King, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Arnd Bergmann,
Krzysztof Kozlowski, Alexandre Belloni, Drew Fustini,
Greg Kroah-Hartman, Jiri Slaby, linux-doc, linux-kernel,
linux-arm-kernel, devicetree, linux-serial
In-Reply-To: <23095518.EfDdHjke4D@silicon.doe.home>
On Fri, May 8, 2026 at 12:09 AM Stefan Dösinger
<stefandoesinger@gmail.com> wrote:
> So I read https://docs.kernel.org/process/maintainer-soc.html a few times. If
> I understand it correctly at this point "pull request" still means emails sent
> with p4, correct?
No it's the contents of an
git request-pull v7.1-rc1 git://..... tags/my-soc
put into a regular email and sent to soc@kernel.org.
I'm sorry if the terminology isn't always clear on what a pull request
actually is in the kernel world (as opposed to e.g. github). It's just
an email with request-pull contents and some cover letter.
> Or does someone create a git repository on git.kernel.org
> for me that I can use to send actual pull requests?
We can pull from wherever as long as you can sign your tag
with a GPG key that we can (in best cases) trust. We can also
just inspect the result of a pull request from a branch (no tag)
if we wanna, it just involves more inspection and trust.
> As I understand it, my 6 patches then go to the 4 corners of the kernel:
>
> Patch 1 (dt binding) to devicetree@vger.kernel.org
Nah as long as the DT maintianers ACK it (i.e. Reviewed-by) we
can merge that to the SoC tree.
> Patches 2 (platform), 5 (DTS) and 6 (defconfig) to soc@kernel.org, but not in
> one series but 3 independent ones
In an ideal world.
> Patches 3 and 4 (UART) to linux-serial@vger.kernel.org. I think this can and
> should be a series of both patches belonging together
Yups. Greg merges those.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v7 4/4] serial: 8250_dw: Use a fixed CPR value for UltraRISC DP1000 UART
From: Jia Wang @ 2026-05-11 9:50 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Jia Wang, Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, LKML,
linux-serial, linux-riscv, devicetree
In-Reply-To: <66f322a7-70c5-13f9-3e97-6d0b18193f91@linux.intel.com>
On 2026-04-29 13:55 +0300, Ilpo Järvinen wrote:
> On Wed, 29 Apr 2026, Jia Wang wrote:
>
> > The UltraRISC DP1000 UART does not provide the standard CPR register used
> > by 8250_dw to discover port capabilities.
> >
> > Provide a fixed CPR value for the DP1000-specific compatible so the
> > driver can configure the port correctly.
> >
> > Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > drivers/tty/serial/8250/8250_dw.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > index 480f82d89856..55e40c10f46a 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -959,6 +959,15 @@ static const struct dw8250_platform_data dw8250_intc10ee = {
> > .quirks = DW_UART_QUIRK_IER_KICK,
> > };
> >
> > +static const struct dw8250_platform_data dw8250_ultrarisc_dp1000_data = {
> > + .usr_reg = DW_UART_USR,
> > + .cpr_value = FIELD_PREP_CONST(DW_UART_CPR_ABP_DATA_WIDTH, 2) |
> > + DW_UART_CPR_THRE_MODE |
> > + DW_UART_CPR_DMA_EXTRA |
> > + DW_UART_CPR_FIFO_MODE_FROM_SIZE(32),
> > + .quirks = DW_UART_QUIRK_CPR_VALUE,
>
> Thanks for all the effort you put to this series,
>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>
> Unrelated to this patch, I suppose we wouldn't strictly need to have
> DW_UART_QUIRK_CPR_VALUE in this driver as non-zero .cpr_value should be
> enough to decide if the CPR quirk should be used or not (if the code is
> adapted, obviously).
>
Thanks for the review and the tag, Ilpo.
The idea of dropping the quirk flag sounds reasonable to me. I'll keep
it in mind for a future cleanup patch once this series is merged.
> > +};
> > +
> > static const struct of_device_id dw8250_of_match[] = {
> > { .compatible = "snps,dw-apb-uart", .data = &dw8250_dw_apb },
> > { .compatible = "cavium,octeon-3860-uart", .data = &dw8250_octeon_3860_data },
> > @@ -966,6 +975,7 @@ static const struct of_device_id dw8250_of_match[] = {
> > { .compatible = "renesas,rzn1-uart", .data = &dw8250_renesas_rzn1_data },
> > { .compatible = "sophgo,sg2044-uart", .data = &dw8250_skip_set_rate_data },
> > { .compatible = "starfive,jh7100-uart", .data = &dw8250_skip_set_rate_data },
> > + { .compatible = "ultrarisc,dp1000-uart", .data = &dw8250_ultrarisc_dp1000_data },
> > { /* Sentinel */ }
> > };
> > MODULE_DEVICE_TABLE(of, dw8250_of_match);
> >
> >
>
> --
> i.
Best Regards,
Jia Wang
^ permalink raw reply
* Re: [PATCH 6.12.y 0/8] serial: 8250_dw: backport BUSY deassert series
From: Ilpo Järvinen @ 2026-05-11 10:26 UTC (permalink / raw)
To: Ionut Nechita (Wind River)
Cc: Greg Kroah-Hartman, Andy Shevchenko, linux-serial
In-Reply-To: <20260510134011.618215-1-ionut.nechita@windriver.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.
^ permalink raw reply
* Re: [PATCH v1 1/1] serial: 8250_fsl: Export fsl8250_handle_irq() conditionally
From: kernel test robot @ 2026-05-11 12:32 UTC (permalink / raw)
To: Andy Shevchenko, linux-serial, linux-kernel
Cc: oe-kbuild-all, Greg Kroah-Hartman
In-Reply-To: <20260504151259.483924-1-andriy.shevchenko@linux.intel.com>
Hi Andy,
kernel test robot noticed the following build errors:
[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next tty/tty-linus linus/master v7.1-rc3 next-20260508]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/serial-8250_fsl-Export-fsl8250_handle_irq-conditionally/20260511-115924
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link: https://lore.kernel.org/r/20260504151259.483924-1-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 1/1] serial: 8250_fsl: Export fsl8250_handle_irq() conditionally
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20260511/202605112047.Ie6qC0oT-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260511/202605112047.Ie6qC0oT-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605112047.Ie6qC0oT-lkp@intel.com/
All errors (new ones prefixed by >>):
arch/powerpc/kernel/legacy_serial.c: In function 'fixup_port_irq':
>> arch/powerpc/kernel/legacy_serial.c:514:44: error: 'fsl8250_handle_irq' undeclared (first use in this function); did you mean 'serial8250_handle_irq'?
514 | port->handle_irq = fsl8250_handle_irq;
| ^~~~~~~~~~~~~~~~~~
| serial8250_handle_irq
arch/powerpc/kernel/legacy_serial.c:514:44: note: each undeclared identifier is reported only once for each function it appears in
vim +514 arch/powerpc/kernel/legacy_serial.c
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 489
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 490 static void __init fixup_port_irq(int index,
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 491 struct device_node *np,
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 492 struct plat_serial8250_port *port)
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 493 {
0ebfff1491ef85 Benjamin Herrenschmidt 2006-07-03 494 unsigned int virq;
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 495
0ebfff1491ef85 Benjamin Herrenschmidt 2006-07-03 496 DBG("fixup_port_irq(%d)\n", index);
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 497
0ebfff1491ef85 Benjamin Herrenschmidt 2006-07-03 498 virq = irq_of_parse_and_map(np, 0);
ef24ba7091517d Michael Ellerman 2016-09-06 499 if (!virq && legacy_serial_infos[index].irq_check_parent) {
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 500 np = of_get_parent(np);
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 501 if (np == NULL)
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 502 return;
0ebfff1491ef85 Benjamin Herrenschmidt 2006-07-03 503 virq = irq_of_parse_and_map(np, 0);
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 504 of_node_put(np);
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 505 }
ef24ba7091517d Michael Ellerman 2016-09-06 506 if (!virq)
0ebfff1491ef85 Benjamin Herrenschmidt 2006-07-03 507 return;
0ebfff1491ef85 Benjamin Herrenschmidt 2006-07-03 508
0ebfff1491ef85 Benjamin Herrenschmidt 2006-07-03 509 port->irq = virq;
9deaa53ac7fa37 Paul Gortmaker 2011-12-04 510
66eff0ef528b6d Uwe Kleine-König 2023-06-05 511 if (IS_ENABLED(CONFIG_SERIAL_8250) &&
66eff0ef528b6d Uwe Kleine-König 2023-06-05 512 of_device_is_compatible(np, "fsl,ns16550")) {
1eea99c04555e5 Uwe Kleine-König 2023-06-09 513 if (IS_REACHABLE(CONFIG_SERIAL_8250_FSL)) {
9deaa53ac7fa37 Paul Gortmaker 2011-12-04 @514 port->handle_irq = fsl8250_handle_irq;
d68fefdd5b5f10 Dmitry Safonov 2019-12-13 515 port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
66eff0ef528b6d Uwe Kleine-König 2023-06-05 516 } else {
66eff0ef528b6d Uwe Kleine-König 2023-06-05 517 pr_warn_once("Not activating Freescale specific workaround for device %pOFP\n",
66eff0ef528b6d Uwe Kleine-König 2023-06-05 518 np);
66eff0ef528b6d Uwe Kleine-König 2023-06-05 519 }
d68fefdd5b5f10 Dmitry Safonov 2019-12-13 520 }
0ebfff1491ef85 Benjamin Herrenschmidt 2006-07-03 521 }
463ce0e103f419 Benjamin Herrenschmidt 2005-11-23 522
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v1 1/1] serial: 8250_fsl: Export fsl8250_handle_irq() conditionally
From: kernel test robot @ 2026-05-11 12:54 UTC (permalink / raw)
To: Andy Shevchenko, linux-serial, linux-kernel
Cc: oe-kbuild-all, Greg Kroah-Hartman
In-Reply-To: <20260504151259.483924-1-andriy.shevchenko@linux.intel.com>
Hi Andy,
kernel test robot noticed the following build errors:
[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next tty/tty-linus linus/master v7.1-rc3 next-20260508]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/serial-8250_fsl-Export-fsl8250_handle_irq-conditionally/20260511-115924
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link: https://lore.kernel.org/r/20260504151259.483924-1-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 1/1] serial: 8250_fsl: Export fsl8250_handle_irq() conditionally
config: microblaze-defconfig (https://download.01.org/0day-ci/archive/20260511/202605112003.uLqRrYzE-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260511/202605112003.uLqRrYzE-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605112003.uLqRrYzE-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/tty/serial/8250/8250_of.c: In function 'of_platform_serial_setup':
>> drivers/tty/serial/8250/8250_of.c:187:36: error: 'fsl8250_handle_irq' undeclared (first use in this function); did you mean 'serial8250_handle_irq'?
187 | port->handle_irq = fsl8250_handle_irq;
| ^~~~~~~~~~~~~~~~~~
| serial8250_handle_irq
drivers/tty/serial/8250/8250_of.c:187:36: note: each undeclared identifier is reported only once for each function it appears in
vim +187 drivers/tty/serial/8250/8250_of.c
32f6ec282fb0ddb drivers/tty/serial/8250/8250_of.c Bastien Curutchet 2024-04-05 83
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 84 /*
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 85 * Fill a struct uart_port for a given device node
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 86 */
9671f09921d93e7 drivers/tty/serial/of_serial.c Bill Pemberton 2012-11-19 87 static int of_platform_serial_setup(struct platform_device *ofdev,
058bc104f7ca5c8 drivers/tty/serial/8250/8250_of.c Lukas Wunner 2020-02-28 88 int type, struct uart_8250_port *up,
0bbeb3c3e84bc96 drivers/tty/serial/of_serial.c Murali Karicheri 2012-10-22 89 struct of_serial_info *info)
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 90 {
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 91 struct resource resource;
4678de73932f8f3 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2023-09-12 92 struct device *dev = &ofdev->dev;
4678de73932f8f3 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2023-09-12 93 struct device_node *np = dev->of_node;
058bc104f7ca5c8 drivers/tty/serial/8250/8250_of.c Lukas Wunner 2020-02-28 94 struct uart_port *port = &up->port;
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 95 u32 spd;
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 96 int ret;
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 97
d3210c8e88ee413 drivers/tty/serial/8250/8250_of.c jempty.liang 2025-11-17 98 memset(port, 0, sizeof(*port));
a2d23edaef9e022 drivers/tty/serial/8250/8250_of.c Franklin S Cooper Jr 2017-08-16 99
a2d23edaef9e022 drivers/tty/serial/8250/8250_of.c Franklin S Cooper Jr 2017-08-16 100 pm_runtime_enable(&ofdev->dev);
a2d23edaef9e022 drivers/tty/serial/8250/8250_of.c Franklin S Cooper Jr 2017-08-16 101 pm_runtime_get_sync(&ofdev->dev);
a2d23edaef9e022 drivers/tty/serial/8250/8250_of.c Franklin S Cooper Jr 2017-08-16 102
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 103 ret = of_address_to_resource(np, 0, &resource);
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 104 if (ret) {
4678de73932f8f3 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2023-09-12 105 dev_err_probe(dev, ret, "invalid address\n");
aef6b8631f9ddf2 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2023-09-19 106 goto err_pmruntime;
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 107 }
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 108
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 109 port->dev = &ofdev->dev;
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 110 port->flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE;
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 111 spin_lock_init(&port->lock);
aa95947400edb57 drivers/tty/serial/8250/8250_of.c John Garry 2018-04-27 112
aa95947400edb57 drivers/tty/serial/8250/8250_of.c John Garry 2018-04-27 113 if (resource_type(&resource) == IORESOURCE_IO) {
aa95947400edb57 drivers/tty/serial/8250/8250_of.c John Garry 2018-04-27 114 port->iobase = resource.start;
aa95947400edb57 drivers/tty/serial/8250/8250_of.c John Garry 2018-04-27 115 } else {
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 116 port->mapbase = resource.start;
0787691230d88af drivers/tty/serial/of_serial.c Mans Rullgard 2015-03-08 117 port->mapsize = resource_size(&resource);
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 118 port->flags |= UPF_IOREMAP;
d06b1cf28297e27 drivers/tty/serial/8250/8250_of.c Robert Hancock 2022-01-12 119 }
d06b1cf28297e27 drivers/tty/serial/8250/8250_of.c Robert Hancock 2022-01-12 120
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 121 ret = uart_read_and_validate_port_properties(port);
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 122 if (ret)
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 123 goto err_pmruntime;
b912b5e2cfb35c0 drivers/serial/of_serial.c John Linn 2008-04-03 124
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 125 /* Get clk rate through clk driver if present */
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 126 if (!port->uartclk) {
81e4de4ba298d73 drivers/tty/serial/8250/8250_of.c Alex Elder 2025-04-09 127 struct clk *bus_clk;
81e4de4ba298d73 drivers/tty/serial/8250/8250_of.c Alex Elder 2025-04-09 128
81e4de4ba298d73 drivers/tty/serial/8250/8250_of.c Alex Elder 2025-04-09 129 bus_clk = devm_clk_get_optional_enabled(dev, "bus");
81e4de4ba298d73 drivers/tty/serial/8250/8250_of.c Alex Elder 2025-04-09 130 if (IS_ERR(bus_clk)) {
81e4de4ba298d73 drivers/tty/serial/8250/8250_of.c Alex Elder 2025-04-09 131 ret = dev_err_probe(dev, PTR_ERR(bus_clk), "failed to get bus clock\n");
81e4de4ba298d73 drivers/tty/serial/8250/8250_of.c Alex Elder 2025-04-09 132 goto err_pmruntime;
81e4de4ba298d73 drivers/tty/serial/8250/8250_of.c Alex Elder 2025-04-09 133 }
81e4de4ba298d73 drivers/tty/serial/8250/8250_of.c Alex Elder 2025-04-09 134
81e4de4ba298d73 drivers/tty/serial/8250/8250_of.c Alex Elder 2025-04-09 135 /* If the bus clock is required, core clock must be named */
81e4de4ba298d73 drivers/tty/serial/8250/8250_of.c Alex Elder 2025-04-09 136 info->clk = devm_clk_get_enabled(dev, bus_clk ? "core" : NULL);
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 137 if (IS_ERR(info->clk)) {
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 138 ret = dev_err_probe(dev, PTR_ERR(info->clk), "failed to get clock\n");
aef6b8631f9ddf2 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2023-09-19 139 goto err_pmruntime;
7423734e19e7e0a drivers/tty/serial/of_serial.c Jamie Iles 2011-06-27 140 }
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 141
926040da6064233 drivers/tty/serial/8250/8250_of.c Alex Elder 2025-04-11 142 info->bus_clk = bus_clk;
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 143 port->uartclk = clk_get_rate(info->clk);
7423734e19e7e0a drivers/tty/serial/of_serial.c Jamie Iles 2011-06-27 144 }
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 145 /* If current-speed was set, then try not to change it. */
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 146 if (of_property_read_u32(np, "current-speed", &spd) == 0)
1117a6fdc7c14d6 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-03-04 147 port->custom_divisor = port->uartclk / (16 * spd);
aa95947400edb57 drivers/tty/serial/8250/8250_of.c John Garry 2018-04-27 148
f4817843e39ce78 drivers/tty/serial/8250/8250_of.c Lubomir Rintel 2019-02-24 149 /* Compatibility with the deprecated pxa driver and 8250_pxa drivers. */
f4817843e39ce78 drivers/tty/serial/8250/8250_of.c Lubomir Rintel 2019-02-24 150 if (of_device_is_compatible(np, "mrvl,mmp-uart"))
f4817843e39ce78 drivers/tty/serial/8250/8250_of.c Lubomir Rintel 2019-02-24 151 port->regshift = 2;
f4817843e39ce78 drivers/tty/serial/8250/8250_of.c Lubomir Rintel 2019-02-24 152
e2860e1f62f2e87 drivers/tty/serial/8250/8250_of.c Joel Stanley 2017-05-29 153 info->rst = devm_reset_control_get_optional_shared(&ofdev->dev, NULL);
b9820a31691b771 drivers/tty/serial/8250/8250_of.c Masahiro Yamada 2017-12-27 154 if (IS_ERR(info->rst)) {
b9820a31691b771 drivers/tty/serial/8250/8250_of.c Masahiro Yamada 2017-12-27 155 ret = PTR_ERR(info->rst);
aef6b8631f9ddf2 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2023-09-19 156 goto err_pmruntime;
b9820a31691b771 drivers/tty/serial/8250/8250_of.c Masahiro Yamada 2017-12-27 157 }
b9820a31691b771 drivers/tty/serial/8250/8250_of.c Masahiro Yamada 2017-12-27 158
e2860e1f62f2e87 drivers/tty/serial/8250/8250_of.c Joel Stanley 2017-05-29 159 ret = reset_control_deassert(info->rst);
e2860e1f62f2e87 drivers/tty/serial/8250/8250_of.c Joel Stanley 2017-05-29 160 if (ret)
aef6b8631f9ddf2 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2023-09-19 161 goto err_pmruntime;
e2860e1f62f2e87 drivers/tty/serial/8250/8250_of.c Joel Stanley 2017-05-29 162
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 163 port->type = type;
283e096ffb7077c drivers/tty/serial/8250/8250_of.c Lukas Wunner 2020-02-28 164 port->rs485_config = serial8250_em485_config;
0139da50dc53f0c drivers/tty/serial/8250/8250_of.c Ilpo Järvinen 2022-07-04 165 port->rs485_supported = serial8250_em485_supported;
058bc104f7ca5c8 drivers/tty/serial/8250/8250_of.c Lukas Wunner 2020-02-28 166 up->rs485_start_tx = serial8250_em485_start_tx;
058bc104f7ca5c8 drivers/tty/serial/8250/8250_of.c Lukas Wunner 2020-02-28 167 up->rs485_stop_tx = serial8250_em485_stop_tx;
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 168
9b8777e3473e31b drivers/tty/serial/of_serial.c John Crispin 2014-10-16 169 switch (type) {
9b8777e3473e31b drivers/tty/serial/of_serial.c John Crispin 2014-10-16 170 case PORT_RT2880:
b334214ea08d941 drivers/tty/serial/8250/8250_of.c Ilpo Järvinen 2023-05-11 171 ret = rt288x_setup(port);
cd0eb354d441488 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-02-15 172 break;
cd0eb354d441488 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-02-15 173 case PORT_NPCM:
cd0eb354d441488 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-02-15 174 ret = npcm_setup(port);
cd0eb354d441488 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-02-15 175 break;
cd0eb354d441488 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-02-15 176 default:
cd0eb354d441488 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-02-15 177 /* Nothing to do */
cd0eb354d441488 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-02-15 178 ret = 0;
9b8777e3473e31b drivers/tty/serial/of_serial.c John Crispin 2014-10-16 179 break;
9b8777e3473e31b drivers/tty/serial/of_serial.c John Crispin 2014-10-16 180 }
cd0eb354d441488 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-02-15 181 if (ret)
cd0eb354d441488 drivers/tty/serial/8250/8250_of.c Andy Shevchenko 2024-02-15 182 goto err_pmruntime;
bf03f65b7967df5 drivers/tty/serial/of_serial.c Dan Williams 2012-04-10 183
d9f59caf94a92f7 drivers/tty/serial/8250/8250_of.c Uwe Kleine-König 2023-06-09 184 if (IS_REACHABLE(CONFIG_SERIAL_8250_FSL) &&
d43b54d269d27bd drivers/tty/serial/of_serial.c Scott Wood 2015-10-07 185 (of_device_is_compatible(np, "fsl,ns16550") ||
d68fefdd5b5f107 drivers/tty/serial/8250/8250_of.c Dmitry Safonov 2019-12-13 186 of_device_is_compatible(np, "fsl,16550-FIFO64"))) {
d43b54d269d27bd drivers/tty/serial/of_serial.c Scott Wood 2015-10-07 @187 port->handle_irq = fsl8250_handle_irq;
d68fefdd5b5f107 drivers/tty/serial/8250/8250_of.c Dmitry Safonov 2019-12-13 188 port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
d68fefdd5b5f107 drivers/tty/serial/8250/8250_of.c Dmitry Safonov 2019-12-13 189 }
d43b54d269d27bd drivers/tty/serial/of_serial.c Scott Wood 2015-10-07 190
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 191 return 0;
a2d23edaef9e022 drivers/tty/serial/8250/8250_of.c Franklin S Cooper Jr 2017-08-16 192 err_pmruntime:
a2d23edaef9e022 drivers/tty/serial/8250/8250_of.c Franklin S Cooper Jr 2017-08-16 193 pm_runtime_put_sync(&ofdev->dev);
a2d23edaef9e022 drivers/tty/serial/8250/8250_of.c Franklin S Cooper Jr 2017-08-16 194 pm_runtime_disable(&ofdev->dev);
0bbeb3c3e84bc96 drivers/tty/serial/of_serial.c Murali Karicheri 2012-10-22 195 return ret;
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 196 }
8d38a5b2fab1397 drivers/serial/of_serial.c Arnd Bergmann 2007-02-13 197
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] tty: n_gsm: fix memory leak in gsm_activate_mux
From: Greg KH @ 2026-05-11 14:44 UTC (permalink / raw)
To: Minu Jin
Cc: jirislaby, daniel.starke, linux-kernel, linux-serial,
syzbot+b5d1f455d385b2c7da3c
In-Reply-To: <20260422183321.596414-1-s9430939@naver.com>
On Thu, Apr 23, 2026 at 03:33:21AM +0900, Minu Jin wrote:
> syzbot reported a memory leak in gsm_activate_mux().
> The root cause is a missing cleanup path when gsm_register_devices()
> fails. In this case, the previously allocated DLCI 0
> and its associated kfifo remain allocated, leading to a memory leak.
>
> And gsm_dlci_alloc() does not check for already allocated DLCIs.
> Repeated calls to gsm_activate_mux() would overwrite the existing pointer
> in gsm->dlci[addr], causing the original memory to be lost.
>
> Fix this by:
> 1. Adding gsm_dlci_free() in the error path of gsm_activate_mux().
> 2. Adding a check in gsm_dlci_alloc() to return the existing DLCI
> if it is already allocated.
>
> Reported-by: syzbot+b5d1f455d385b2c7da3c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b5d1f455d385b2c7da3c
> Tested-by: syzbot+b5d1f455d385b2c7da3c@syzkaller.appspotmail.com
> Fixes: 01aecd917114 ("tty: n_gsm: fix tty registration before control channel open")
> Signed-off-by: Minu Jin <s9430939@naver.com>
> ---
> drivers/tty/n_gsm.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index c13e050de83b..de3d30eac86e 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -2645,7 +2645,12 @@ static int gsm_dlci_config(struct gsm_dlci *dlci, struct gsm_dlci_config *dc, in
>
> static struct gsm_dlci *gsm_dlci_alloc(struct gsm_mux *gsm, int addr)
> {
> - struct gsm_dlci *dlci = kzalloc_obj(struct gsm_dlci, GFP_ATOMIC);
> + struct gsm_dlci *dlci;
> +
> + if (gsm->dlci[addr])
> + return gsm->dlci[addr];
Why would you be allocating a device twice? Shouldn't that logic be
fixed instead?
> +
> + dlci = kzalloc_obj(struct gsm_dlci, GFP_ATOMIC);
> if (dlci == NULL)
> return NULL;
> spin_lock_init(&dlci->lock);
> @@ -3196,8 +3201,10 @@ static int gsm_activate_mux(struct gsm_mux *gsm)
> gsm->receive = gsm1_receive;
>
> ret = gsm_register_devices(gsm_tty_driver, gsm->num);
> - if (ret)
> + if (ret) {
> + gsm_dlci_free(&dlci->port);
> return ret;
> + }
How was this tested?
thanks,
greg k-h
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox